feat: Simplifying error handling a little bit #2504
Conversation
Signed-off-by: Thor Anker Kvisgård Lange <tal@netic.dk>
|
Deployment of preview was successful: https://woodpecker-ci-woodpecker-pr-2504.surge.sh |
| err := ErrFilteredRestrictions | ||
| log.Debug().Str("repo", repo.FullName).Msgf("%v", err) | ||
| return nil, err |
There was a problem hiding this comment.
I think you can replace here (and below) too the local var err directly with ErrFilteredRestrictions
|
@langecode you also need to format your code: https://ci.woodpecker-ci.org/repos/3780/pipeline/8405/28 |
|
Except the test you added this is included in #2527 |
Yes, I know - we forked the repo prior to #2327 and discovered that the webhook was returning strange http response codes to the git server when pipelines were skipped. That is also why I thought you could take it or leave it - feel free to close the PR if you want. I think I also mentioned in the initial text that it was already fixed in The reason why I chose to propose it anyway is that I think introducing a separate type here, that can ever only hold two different values is somewhat more complex to have two "constants" with the two errors. Had the error struct contained more dynamic information it may have been more relevant - but going through the code it is only ever instantiated with two different constant strings. |
|
Test extracted to #2547 |
Credits: @langecode Taken from #2504
During work on Bitbucket Server implementation we discovered that skipped pipelines yielded a wrong responses back to the Bitbucket webhook. We tracked this down to the error handling.
I notice that this has actually also been changed on
mainsince we originally forked the project to work on the Bitbucket Server issues. However since the filtering error only ever occur with two different messages I think the solution with creating two constant error instances might be simpler anyway. You can take it or leave it :)