-
Notifications
You must be signed in to change notification settings - Fork 3k
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
mergify: enable for any pull request #12274
Conversation
This reverts commit e4cd938.
Notes:
This should minimize the time between workflow stages. If you find an error, please review "Checks" - Mergify rules and let us know what is not working, or just fix it based on the Mergify documentation. This PR won't have mergify bot active (see 2nd note). Once merged, it will start. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing a state:
In 'needs work' and new commits are made --> needs review ?
@@ -53,7 +51,6 @@ pull_request_rules: | |||
# From needs: review to needs: work - CI failure | |||
- name: "label needs: work when Jenkins CI failed - pr head" | |||
conditions: | |||
- base~=feature-mergify |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we also have a needs: ci label here that may need to be removed if travis was restarted during a CI cycle ?
@@ -66,7 +63,6 @@ pull_request_rules: | |||
# From needs: review to needs: work - CI failure | |||
- name: "label needs: work when Jenkins CI failed - any of the pipeline" | |||
conditions: | |||
- base~=feature-mergify |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, may need to also remove needs: ci if present
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor changes needed
I added CI label removal.
There is no condition for this at the moment, will check and request this one. |
There is this condition: https://doc.mergify.io/examples.html#removing-stale-reviews (not yet complete, see below). there's an issue about similar functionality we want: Mergifyio/mergify#360. I've left there a comment about what can be done. |
I updated stale reviews update. I added this:
Means after the branch is updated, it will dismissed approved/requested changes and send a message about the update. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
CI started |
One additional stage there - thinking about the updating PR after needs: work state. As we have jenkins pr-head there and its yellow whenever PR is created or updated. It means updated PR is when pr-head is yellow (no success or failure) and PR is in "needs: work" state. Action would be to move from needs work to needs review. This should go along with dismissed reviews |
Summary of changes
Add mergify bot to all PRs (all branches). The configuration already resides on master but not yet enabled, only been enabled for feature-mergify to test it.
Docs: https://doc.mergify.io/
The configuration should follow our workflow. If it does not, we should fix via new pull request anytime (note: one drawback, the config is fetched from the default branch, in our case master. So even rules for non master, need to be in the config there). The auto merge is disabled, the bot does not have write access to any of the branches anyway. This bot can move labels, post comments.
To verify for each PR, use "Checks" where you can review Mergify conditions.
To disable, just revert this comment - to have all rules only on feature-mergify for instance. Hopefully, this won't be necessary and we tune the rules once we get more PRs involved (compare to few testing we did).
Impact of changes
Migration actions required
Documentation
Pull request type
Test results
Reviewers
@bulislaw