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

[BUG] CHANGELOG is incorrect across branches (main vs. 2.x) #4936

Closed
dblock opened this issue Oct 26, 2022 · 10 comments
Closed

[BUG] CHANGELOG is incorrect across branches (main vs. 2.x) #4936

dblock opened this issue Oct 26, 2022 · 10 comments
Assignees
Labels
bug Something isn't working

Comments

@dblock
Copy link
Member

dblock commented Oct 26, 2022

Describe the bug

The CHANGELOG on main contains a 2.x section, which makes no sense because all changes on main are 3.0. So the only section that this CHANGELOG should have is unreleased (3.x). The CHANGELOG on 2.x contains a 2.x section, which is also incorrect. On 2.x it should be all changes that went onto 2.x.

Expected behavior

CHANGELOG on main = all changes in 3.0
CHANGELOG on 2.x = all changes on 2.x

I believe we have made a mess by not paying attention. Most changes on 2.x are backports, and each changelog line should be the change that has been backported, not the change on main. The auto-backport workflow likely needs to exclude CHANGELOG in the backport and add a backport line, amend the commit with the PR number and push.

@dblock dblock added bug Something isn't working untriaged labels Oct 26, 2022
@dblock
Copy link
Member Author

dblock commented Oct 26, 2022

@kotwanikunal LMK if you want to/can take this? I think whatever we are doing isn't working :(

@kotwanikunal
Copy link
Member

@kotwanikunal LMK if you want to/can take this? I think whatever we are doing isn't working :(

Let me take a look at this.
#1868 also has a lot of discussion recently. Let me summarize and move that forward as well.

@andrross
Copy link
Member

@dblock Just to clarify on the expected behavior, does this mean that if I commit a change to main that I intend to backport to 2.x, then the commit on main should not modify the CHANGELOG file and only the backport should modify the CHANGELOG file? And the only time I would modify the CHANGELOG file on main would be when I don't intend to backport it (because it will remain unreleased until the first 3.x release)?

@kotwanikunal
Copy link
Member

I had a look at this. I think backporting is creating the biggest issue with achieving this independency.
I was able to successfully test some changes to the existing backport workflow here.
I have reached out to Vacha for a release.
Post that I will clean up the versioning mess.

Sample PRs with the above change:
Original PR: kotwanikunal#40
Backport: kotwanikunal#41
Skips changelog file

@dblock
Copy link
Member Author

dblock commented Oct 28, 2022

@dblock Just to clarify on the expected behavior, does this mean that if I commit a change to main that I intend to backport to 2.x, then the commit on main should not modify the CHANGELOG file and only the backport should modify the CHANGELOG file? And the only time I would modify the CHANGELOG file on main would be when I don't intend to backport it (because it will remain unreleased until the first 3.x release)?

Well, that's a good question. From the end user POV I think that's exactly right, if the feature shipped in 2.4.0 then there's no reason to call it out in 3.0.0. The only way I see to make it happen right now would be to do passes on the 3.0 CHANGELOG and dedup. This isn't great.

@dblock
Copy link
Member Author

dblock commented Oct 28, 2022

@kotwanikunal In your example I think we have the opposite problem: the backport to 2.x is where we want the changelog because that's the version in which it's going to ship. And on main we don't want it because it will ship in 2.x. I think we made a mistake and this approach is not working :(

@kotwanikunal
Copy link
Member

@kotwanikunal In your example I think we have the opposite problem: the backport to 2.x is where we want the changelog because that's the version in which it's going to ship. And on main we don't want it because it will ship in 2.x. I think we made a mistake and this approach is not working :(

@dblock @andrross

So an update on the approach that I was suggesting -

  1. The backport workflow skips CHANGELOG.md which was done as a part of the original PR here - Fix backport issues for CHANGELOG.md file #4977
  2. We add in a changelog helper (POC for that here) to the backport workflow to do the following -
    a. Extract the changelog line number from original PR using git diff endpoint
    b. Format the changelog line as per the current PR, add it to the appropriate section, version with the backport PR #

I was able to use the POC to achieve the overall backport with changelog without any additional user input. It's done all by the workflow.

Original PR: kotwanikunal#69
Backport PR: kotwanikunal#70
Let me know if this sounds like a good plan going forward. Otherwise we can rollback all of the CHANGELOG process and look for a better approach.

@andrross
Copy link
Member

andrross commented Nov 1, 2022

@kotwanikunal In your example above, the end state is that main has the changelog entry in "unreleased" and the backport branch has the entry in "2.x". Is that right? I think what we want is that the backport branch has the entry, and main has no entry (assuming the backport version gets released).

@kotwanikunal
Copy link
Member

We can potentially tweak the plugin to achieve that. It will raise a PR simultaneously to remove the entry from the original changelog.

@dblock
Copy link
Member Author

dblock commented Nov 2, 2022

I think I like it @kotwanikunal as a next step because at least it fixes known problems. So let's do it. We should label changelog backport PRs to skip CI.

In parallel maybe we should explore automatically generated changelogs on every merged PR from PR titles and enforcing some format thereof suggested by @seraphjiang, but if the solution you propose works well, then we're all set.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants