Skip to content

Conversation

@57300
Copy link
Contributor

@57300 57300 commented Oct 9, 2024

fixup! [nrf noup] ci: Enable action-manifest-pr

When the PR title or body contains "manifest-pr-skip", this action will cancel itself, which results in a failure status. Use the same condition to skip the whole workflow instead.

@57300 57300 force-pushed the fix-manifest-pr-skip branch 6 times, most recently from d2c0cfd to 93e8101 Compare October 9, 2024 04:56
fixup! [nrf noup] ci: Enable action-manifest-pr

When the PR title or body contains "manifest-pr-skip", this action will
cancel itself, which results in a failure status. Use the same condition
to skip the whole workflow instead.

Signed-off-by: Grzegorz Swiderski <[email protected]>
@57300 57300 force-pushed the fix-manifest-pr-skip branch from 93e8101 to 6cdd669 Compare October 9, 2024 05:02
@57300 57300 marked this pull request as ready for review October 9, 2024 05:05
@57300 57300 requested a review from rugeGerritsen October 9, 2024 05:05
Comment on lines +14 to +16
if: >
!(contains(github.event.pull_request.title, 'manifest-pr-skip') ||
contains(github.event.pull_request.body, 'manifest-pr-skip'))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hopefully it works, but I don't know how to verify it in advance.

Copy link
Contributor

Choose a reason for hiding this comment

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

@57300 . I believe this should rather be fixed in the action itself, so that it also works for other users of this action. @karhama , do you agree?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes I would recommend changing action itself if cancelled status is causing problems. However I did not fully understand in which case cancelled becomes the problem? Idea in action side was roughly : success == manifest PR created, failure == failed to create manifest PR (when it should have been created) and cancelled == "neutral status" i.e. no need to create manifest PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updating the action itself sounds reasonable. I'm just unsure what's the best way to do it, seems to me like it could make the code more convoluted.

The only problem with the cancel is that it makes the PR red, which is misleading. It also shows up when filtering PRs by "status:failure". I think a code owner would still be able to merge it though, so I guess it's minor.

@57300
Copy link
Contributor Author

57300 commented Oct 16, 2024

Resolved via nrfconnect/action-manifest-pr#13

@57300 57300 closed this Oct 16, 2024
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.

5 participants