fix: Revert GitOps event_type to pull_request #1729
Conversation
|
/retest |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1729 +/- ##
==========================================
- Coverage 64.65% 64.64% -0.02%
==========================================
Files 145 146 +1
Lines 11195 11265 +70
==========================================
+ Hits 7238 7282 +44
- Misses 3428 3452 +24
- Partials 529 531 +2 ☔ View full report in Codecov by Sentry. |
|
|
||
| ## Event Type Annotation and dynamic variables | ||
|
|
||
| The `pipeline.tekton.dev/event-type` annotation on labels indicates the type of |
There was a problem hiding this comment.
What is "annotation on labels"?
"...GitOps command which triggered the pipelinerun" sounds simpler to me.
There was a problem hiding this comment.
@chmouel verified below scenarios
- For all the
/test,/retest,/testand/retest ` i see event_type as pull_request - For
on-commentannotationsevent_typeis no-ops-comment - For invalid gitops comments also event_type is no-ops-comment
- I also see deprecation notice in the controller log as well as in the events
<unknown> Warning DeprecatedOpsComment repository/savitaashture-test-pac the test-all-comment event type is deprecated, this will be changed to pull_request in the future
<unknown> Warning DeprecatedOpsComment repository/savitaashture-test-pac the retest-comment event type is deprecated, this will be changed to pull_request in the future
| // we keep on-comment to the "on-comment" type | ||
| func EventTypeBackwardCompat(eventEmitter *events.EventEmitter, repo *v1alpha1.Repository, label string) string { | ||
| if label == OnCommentEventType.String() { | ||
| return label |
There was a problem hiding this comment.
Do we consider on-comment also as no-ops-comment ???
Asking because when i specify pipelinesascode.tekton.dev/on-comment: "^/hello-world" in PipelineRun yaml and print {{ event_type }} i get no-ops-comment and even for non on-comment gitops comments like /savita i get {{ event_type }} as no-ops-comment
There was a problem hiding this comment.
@chmouel how about this ???
There was a problem hiding this comment.
ah weird ! does that happen before this pr as well?
There was a problem hiding this comment.
Ah actually i did not check that
There was a problem hiding this comment.
I tested it and can reproduce on the main branch too and created a jira issue for it... will try to do it asap but we can still merge this PR since on-comments is TP
- Reverted the following event types to 'pull_request': * NoOpsCommentEventType * TestAllCommentEventType * TestSingleCommentEventType * RetestSingleCommentEventType * RetestAllCommentEventType * OnCommentEventType * CancelCommentSingleEventType * CancelCommentAllEventType * OkToTestCommentEventType - This change restores compatibility for users relying on 'pull_request' event type for filtering GitOps comments. - Added deprecation notice in documentation for future changes to event types. - Added deprecation notice in the events stream when deprecated events comment is used. - Planned deprecation period to be determined and communicated in future updates. Breaking Changes: - None. This change restores previous behavior. Deprecated: - Specific event types for GitOps comments (to be deprecated in future release). TODO: - Plan and communicate deprecation timeline for specific GitOps comment event types. - Update documentation with clear migration path for future changes. Fixes https://issues.redhat.com/browse/SRVKP-5775 Signed-off-by: Chmouel Boudjnah <[email protected]>
chmouel
force-pushed
the
SRVKP-5775-backward-compat-gitops-comments-event-type
branch
from
58dc16a
to
4484a57
Compare
|
@chmouel can we add e2e test to verify event_type |
Okok sorry missed that |
|
Thank you /lgtm |
Reverted the following event types to 'pull_request':
This change restores compatibility for users relying on
'pull_request' event type for filtering GitOps comments.
Added deprecation notice in documentation for future changes to event types.
Added deprecation notice in the events stream when deprecated events
comment is used.
Planned deprecation period to be determined and communicated in future
updates.
Breaking Changes:
Deprecated:
TODO:
event types.
Fixes https://issues.redhat.com/browse/SRVKP-5775
Signed-off-by: Chmouel Boudjnah [email protected]
Submitter Checklist
📝 Please ensure your commit message is clear and informative. For guidance on crafting effective commit messages, refer to the How to write a git commit message guide. We prefer the commit message to be included in the PR body itself rather than a link to an external website (ie: Jira ticket).
♽ Before submitting a PR, run make test lint to avoid unnecessary CI processing. For an even more efficient workflow, consider installing pre-commit and running pre-commit install in the root of this repository.
✨ We use linters to maintain clean and consistent code. Please ensure you've run make lint before submitting a PR. Some linters offer a --fix mode, which can be executed with the command make fix-linters (ensure markdownlint and golangci-lint tools are installed first).
📖 If you're introducing a user-facing feature or changing existing behavior, please ensure it's properly documented.
🧪 While 100% coverage isn't a requirement, we encourage unit tests for any code changes where possible.
🎁 If feasible, please check if an end-to-end test can be added. See README for more details.
🔎 If there's any flakiness in the CI tests, don't necessarily ignore it. It's better to address the issue before merging, or provide a valid reason to bypass it if fixing isn't possible (e.g., token rate limitations).