Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not error when no matching pipeline can be found #690

Merged
merged 1 commit into from
May 12, 2023

Conversation

michaelsauter
Copy link
Member

@michaelsauter michaelsauter commented May 12, 2023

Returning an error does not seem appropriate because there can be situations in which no pipeline should match which are "expected". For example, one might configure to act on PR comments of "/deploy", but for all other PR comment (events), nothing should happen.

The log will still report that there was no matching pipeline so that users can figure out what is going on.

The issue was introduced in #685. Since that has not been released yet, the bugfix will not be recorded in the changelog.

Tasks:

  • Updated design documents in docs/design directory or not applicable
  • Updated user-facing documentation in docs directory or not applicable
  • Ran tests (e.g. make test) or not applicable
  • Updated changelog or not applicable

Returning an error does not seem appropriate because there can be
situations in which no pipeline should match which are "expected". For
example, one might configure to act on PR comments of "/deploy", but for
all other PR comment (events), nothing should happen.

The log will still report that there was no matching pipeline so that
users can figure out what is going on.

The issue was introduced in #685. Since that has not been released yet,
the bugfix will not be recorded in the changelog.
@michaelsauter michaelsauter force-pushed the fix/non-matching-pipeline-error branch from dc8e614 to b830778 Compare May 12, 2023 12:12
@michaelsauter michaelsauter requested review from kuebler and henrjk May 12, 2023 12:12
Copy link
Member

@henrjk henrjk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -142,6 +142,8 @@ The following selection criteria may be specified:
`exceptTags`:: List of tags to which the triggering event may not refer. Patterns as supported by link:https://pkg.go.dev/path#Match[`path.Match`] may be used to match the excluded tags. Omitting the criterion will lead to none of the tags referred to in the webhook event to be excluded.
`prComment`:: Define a prefix a comment has to start with. Might be used to implement functionality like slash commands. If omitted, comments won't be considered in the pipeline selection process.

CAUTION: link:https://pkg.go.dev/path#Match[`path.Match`] does not match e.g. `feature/foo` when the pattern is just `\*`. If you want to match strings with slashes, specify the pattern `*/\*` as well. For example, to match all branches, write `branches: ["*", "\*/*"]`.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #691

@michaelsauter michaelsauter merged commit 4af2a7b into master May 12, 2023
@michaelsauter michaelsauter deleted the fix/non-matching-pipeline-error branch May 12, 2023 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants