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

Fix backport issues for CHANGELOG.md file #4977

Merged
merged 1 commit into from
Oct 28, 2022

Conversation

kotwanikunal
Copy link
Member

Signed-off-by: Kunal Kotwani [email protected]

Description

  • Fixes the issues caused by CHANGELOG merge conflicts when using the backport GHA

Issues Resolved

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@kotwanikunal kotwanikunal requested review from a team and reta as code owners October 28, 2022 18:34
@kotwanikunal
Copy link
Member Author

@dblock @VachaShah

@@ -22,7 +22,8 @@ jobs:
installation_id: 22958780

- name: Backport
uses: VachaShah/[email protected]
uses: VachaShah/[email protected]
with:
github_token: ${{ steps.github_app_token.outputs.token }}
branch_name: backport/backport-${{ github.event.number }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be head_template: backport/backport-<%= number %>-to-<%= base %> instead of branch_name.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

Copy link
Collaborator

@VachaShah VachaShah left a comment

Choose a reason for hiding this comment

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

LGTM!

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@CEHENKLE CEHENKLE merged commit 171d141 into opensearch-project:main Oct 28, 2022
@kotwanikunal kotwanikunal added the backport 2.x Backport to 2.x branch label Oct 28, 2022
opensearch-trigger-bot bot pushed a commit that referenced this pull request Oct 28, 2022
Signed-off-by: Kunal Kotwani <[email protected]>

Signed-off-by: Kunal Kotwani <[email protected]>
(cherry picked from commit 171d141)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md
@dblock
Copy link
Member

dblock commented Oct 28, 2022

With this change backports no longer have an entry in the CHANGELOG for changes. This means the 2.4.0 changelog will not include all the changes. I think this is ... a problem.

@kotwanikunal
Copy link
Member Author

With this change backports no longer have an entry in the CHANGELOG for changes. This means the 2.4.0 changelog will not include all the changes. I think this is ... a problem.

We were constantly having issues with the backport GHA because of CHANGELOG which was causing oncall pain due to manual backports. There was also an issue of missing out on backport for certain versions considering the human effort involved. This process gets the workflow back to its original state, the backport PR will have the changelog verifier step fail, which can be fixed pretty easily by adding in the appropriate entry.

When backporting, folks were not moving the change to the appropriate section. This will take care of that as well.
I am also working to add a new helper workflow soon which will backport the actual line of change from the original PR to the backported PR, in the appropriate section and version.

@kotwanikunal kotwanikunal deleted the fix-backport branch October 31, 2022 20:04
@dblock
Copy link
Member

dblock commented Nov 1, 2022

I see, thanks. The backport automation is becoming a lot less automated without the implementation you suggest in the last paragraph. For project maintainers it's not too hard to amend a PR, but for contributors it means mostly manually re-creating the backport in a fork?

@kotwanikunal
Copy link
Member Author

I see, thanks. The backport automation is becoming a lot less automated without the implementation you suggest in the last paragraph. For project maintainers it's not too hard to amend a PR, but for contributors it means mostly manually re-creating the backport in a fork?

Please see - #4936 (comment) 🙂
This use case only walks through backporting. Ideally, we do not want contributors to re-create a backport PR (which is already happening because of changelog conflicts).

The comment linked has a solution to both the aforementioned problems. It does the backporting without the changelog file, then backports the actual changelog line, all in a single workflow.

kotwanikunal pushed a commit that referenced this pull request Nov 4, 2022
Signed-off-by: Kunal Kotwani <[email protected]>

Signed-off-by: Kunal Kotwani <[email protected]>
(cherry picked from commit 171d141)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md
dblock pushed a commit that referenced this pull request Nov 8, 2022
Signed-off-by: Kunal Kotwani <[email protected]>

Signed-off-by: Kunal Kotwani <[email protected]>
(cherry picked from commit 171d141)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants