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

Automatically copy changelog message to the backport or put no-changelog label #195

Merged
merged 1 commit into from
Dec 6, 2023

Conversation

AntonAM
Copy link
Contributor

@AntonAM AntonAM commented Dec 6, 2023

With the new requirement of specifying changelog in the PR body or no-changelog label there's a problem with automatic backports, since they automatically fail those requirements. This PR improves backporting procedure by copying changelog message from the parent PR if it's present, or setting no-changelog label.

@AntonAM AntonAM force-pushed the anton/autobackport-changelog branch from de142b8 to f76b337 Compare December 6, 2023 07:45
@AntonAM AntonAM marked this pull request as ready for review December 6, 2023 07:45
@AntonAM AntonAM requested review from a team, fheinecke and r0mant December 6, 2023 07:45

// If automatic backport body doesn't have changelog entry it means there shouldn't be one, otherwise
// it would be automatically added. Mark PR with no-changelog label.
if len(b.getChangelogEntries(prBody)) == 0 && strings.HasPrefix(b.c.Environment.UnsafeHead, botBackportBranchPrefix) {
Copy link
Contributor

Choose a reason for hiding this comment

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

As an alternative to this, should we just copy all labels (except the backport labels). That way if no-changelog (and other attributes) are set on the original PR, they are defaulted to the backport.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed any changes to the label workflow in this PR, now setting the label on the backport workflow itself, as @r0mant suggested. I think we can actually do copying of all labels on the backport workflow, but I feel like that should be a separate PR if we decide to do that.

@@ -117,7 +131,7 @@ func (b *Bot) Backport(ctx context.Context) error {
RawQuery: url.Values{
"expand": []string{"1"},
"title": []string{fmt.Sprintf("[%v] %v", strings.Trim(base, "branch/"), pull.UnsafeTitle)},
"body": []string{fmt.Sprintf("Backport #%v to %v", b.c.Environment.Number, base)},
"body": []string{bodyText},
Copy link
Contributor

Choose a reason for hiding this comment

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

You can just add "labels" here than modifying labels.go, would be a little bit cleaner. I actually had this logic implemented and sitting in #148 for a while but never got a chance to wrap it up.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, damn, I tested it and it didn't work for me, now I tried again and it works, because first time I tried it with test label being "label1" and it probably didn't put it because such label doesn't exist 🤦
Will change to setting label here 👍

Copy link
Contributor Author

@AntonAM AntonAM Dec 6, 2023

Choose a reason for hiding this comment

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

Changed. I had to force push to overwrite the commit, because during writing tests for backport flow it accidentally ran real git commands on my machine and overwrote my git user, which I didn't notice until now (it's safe now, dry run in tests).

@AntonAM AntonAM force-pushed the anton/autobackport-changelog branch from f76b337 to 7fdf32c Compare December 6, 2023 18:23
Copy link
Contributor

@r0mant r0mant left a comment

Choose a reason for hiding this comment

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

Thanks @AntonAM! 🙌

@AntonAM AntonAM force-pushed the anton/autobackport-changelog branch from 7fdf32c to 07a5800 Compare December 6, 2023 18:39
@AntonAM AntonAM merged commit d06c6cc into main Dec 6, 2023
8 checks passed
@fheinecke
Copy link
Contributor

Thanks @AntonAM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants