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.

Sign up for GitHub

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

Jump to bottom

[mergify] backport automation to 7.x, 7.12 and 7.11 #24608

Merged
merged 8 commits into from
Apr 7, 2021
Merged

[mergify] backport automation to 7.x, 7.12 and 7.11 #24608

Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
da361cc
[mergify] backport automation to 7.x, 7.12 and 7.11
v1v Mar 17, 2021
edc1caa
Use current labels
v1v Mar 18, 2021
bab54e9
Add ask to resolve conflict with the how to
v1v Mar 18, 2021
254b2dd
cosmetic change to validate the mergify
v1v Mar 23, 2021
840d99a
quote the branches
v1v Mar 23, 2021
c1238b8
Merge remote-tracking branch 'upstream/master' into feature/mergify
v1v Apr 7, 2021
bf22c7e
[mergify] use the merged condition
v1v Apr 7, 2021
ff1eaca
Use backport-vMajor.Minor.0 labels for the experimental phase
v1v Apr 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .mergify.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
pull_request_rules:
- name: backport patches to 7.x branch
conditions:
- merged
- base=master
- label=v7.13.0
v1v marked this conversation as resolved.
Show resolved Hide resolved
actions:
Expand All @@ -9,6 +10,7 @@ pull_request_rules:
- "7.x"
- name: backport patches to 7.12 branch
conditions:
- merged
- base=master
- label=v7.12.0
Copy link

Choose a reason for hiding this comment

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

How exactly will the labels work?

a. If we are about to release 7.12.0, I might want to use the label v7.12.1, in order to postpone the backport for after the release is out. Will we update the version filter here after a release? If so, will my PR automatically backported when we update the label filter here?

b. Will my PR be backported if I add v7.12.0 even after lets say 7.12.1 was released? (Although it might be a bit confusing to users looking at the PR labels to figure out if a bug fix is available)

Copy link
Member Author

@v1v v1v Mar 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I'll answer your questions in a minute, for clarity, the initial proposal was to use the existing labels vmajor.minor.0 (see #24576 (comment))

a. If we are about to release 7.12.0, I might want to use the label v7.12.1, in order to postpone the backport for after the release is out. Will we update the version filter here after a release? If so, will my PR automatically backported when we update the label filter here?

Yes, mergify works for even PRs which were not backported after the merge, as long as the label is added. In addition you can use the github comment = @Mergifyio backport <destination> will backport this PR on <destination> branch

To illustrate the above, I just tagged elastic/apm-pipeline-library#1053 with the explicit label and elastic/apm-pipeline-library#1056 was created. NOTE: for some reason the timestamp for the label does not match with when I did it, (about 7 minutes ago instead of 2 days...)

b. Will my PR be backported if I add v7.12.0 even after lets say 7.12.1 was released? (Although it might be a bit confusing to users looking at the PR labels to figure out if a bug fix is available)

Yes, backports with mergify don't know about releases but branches and labels.

Copy link
Contributor

Choose a reason for hiding this comment

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

a. If we are about to release 7.12.0, I might want to use the label v7.12.1, in order to postpone the backport for after the release is out. Will we update the version filter here after a release? If so, will my PR automatically backported when we update the label filter here?

if the mergify config is updated with that branch before you merge the PR, the PR would backported.

b. Will my PR be backported if I add v7.12.0 even after lets say 7.12.1 was released? (Although it might be a bit confusing to users looking at the PR labels to figure out if a bug fix is available)

the backport is made on a new PR when you merge the fixed PR, if the labels and mergify configuration are in place when you merge, a new PR is made to every branch that you requested backport with the labels.

There is an alternative way to make the backports manually using a GitHub comment (see https://docs.mergify.io/commands.html#backport)

https://mergify.io/features/backports

Copy link

Choose a reason for hiding this comment

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

Maybe to clarify case a:

  1. mergify only backports if label v7.12.0 is set
  2. I create a PR and merge it
  3. I add the label v7.12.1 to my PR (because of FF I don't want the change be backported yet)
  4. Version 7.12.0 gets released
  5. After 7.12.0 is released we update the mergify configuration to backport if v7.12.0 or v7.12.1 is set.

After step 5, will my old PR be picked up automatically or am I required to remember that I did have a 2 weeks old PR that I must now 'manually' backport by adding a comment to the PR (or remove + add label again).

Case b is a little unfortunate as labels do not necesarrily reflect reality, but well, it's the way it is.

Copy link
Contributor

Choose a reason for hiding this comment

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

After 7.12.0 is released we update the mergify configuration to backport if v7.12.0 or v7.12.1 is set.

this happens after you merge the PR, mergify does not review old merged PRs, so the backport is not made. In this case, you have to manually backport the PR, either from the PR with a GiHub Comment or as you make now from your laptop.
However, if we detect changes on the mergify configfile can trigger something that reviews all PRs with the new label and make backport mergify comments on those PRs, but this is something not out of the box. We have to see if this use case exists and how many times it happens, then we can evaluate the effort and the value of this process.

Copy link
Contributor

@kuisathaverat kuisathaverat Mar 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

AFAIK if you fix something in branch 7.12 that has the version 7.12.0 this will be in the 7.12 branch (the same major.minor) that will contain the version 7.12.1 (different patch), so in the case you put as an example there is no backport.
I can see a case similar of what you explain between minor versions, but in those case you use the branch 7.x and then you backport to 7.12, 7.11, ... in those cases the branch already exists so the backport is made.

Copy link
Member Author

Choose a reason for hiding this comment

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

To help to unblock this 🧵

Since there is no backport automation in place for this project I see the benefits of enabling it for facilitating the backporting process, but I don't know if it could cause any issues. Although, mergify requires certain discipline regarding what labels should be added.

Do you think if we enable mergify it will help with the backporting? Or is it required to solve all of the above-mentioned use cases?

Copy link

@urso urso Mar 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think either behavior is a blocker and adding mergify is an improvement either way. As we have to document the backport process in our how to guides. I'm trying to figure out how it works and where we have potential gotchas that we should document as well. I did just bring up the two cases as we've seen them from time to time in the past: label does not match actual release + us holding of with backport in order to target the next patch release... cough... kafka ... cough.

v1v marked this conversation as resolved.
Show resolved Hide resolved
actions:
Expand All @@ -17,6 +19,7 @@ pull_request_rules:
- "7.12"
- name: backport patches to 7.11 branch
conditions:
- merged
- base=master
- label=v7.11.0
v1v marked this conversation as resolved.
Show resolved Hide resolved
actions:
Expand All @@ -29,7 +32,7 @@ pull_request_rules:
actions:
comment:
message: |
This pull request is now in conflicts. Could you fix it @{{author}}? 🙏
This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/
```
git fetch upstream
Copy link
Member

@sorenlouv sorenlouv Apr 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Wouldn't it be easier to ask the user to use a tool for this? Either running mergify locally or backport.
In Kibana we help the user like this:
image

Copy link
Member Author

Choose a reason for hiding this comment

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

good point. I don't know what's the tool the team have in place for that, in order to be be tool-agnostic the message provides the git commands to solve it locally.

I'll keep a note to iterate on this and make the user experience smooth. Thanks for the feedback :)

Copy link

Choose a reason for hiding this comment

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

Most people use the dev-tools/cherry-pick-pr tool in the Beats repo it seems. But a few contributors use the nodejs backport tool. Both are supported. For the changelog we have a 'requirement' on the PR title to properly link to the original PR, so we can find unique PRs and double check if these features are really released (github tags can be missleading). Both tools are supported by our changelog scripts. Using custom git commands might be a problem if the title is not properly formatted (unless we add another lint check on the PR title for backports).

Copy link
Member Author

Choose a reason for hiding this comment

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

We can then iterate with the existing tools, the current comment intends to help with fixing the just created backport PR that has got some conflicts, so we might need to think about that particular use case. I'll keep a note for the upcoming iterations

Expand Down