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

Update changelog readme to add rules on how to add changelog items #14570

Merged
merged 1 commit into from
Oct 14, 2022

Conversation

@codecov-commenter
Copy link

codecov-commenter commented Oct 10, 2022

Codecov Report

Merging #14570 (920eb20) into main (920eb20) will not change coverage.
The diff coverage is n/a.

❗ Current head 920eb20 differs from pull request most recent head 6718031. Consider uploading reports for the commit 6718031 to get more accurate results

@@           Coverage Diff           @@
##             main   #14570   +/-   ##
=======================================
  Coverage   75.49%   75.49%           
=======================================
  Files         457      457           
  Lines       37190    37190           
=======================================
  Hits        28077    28077           
  Misses       7367     7367           
  Partials     1746     1746           
Flag Coverage Δ
all 75.49% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@@ -9,3 +11,11 @@ If etcd process is killed, occasionally some committed transactions are not refl
Recommendation is to upgrade to v3.5.4+.

If you have encountered data corruption, please follow instructions on https://etcd.io/docs/v3.5/op-guide/data_corruption/.

## Change log rules
Copy link
Member

Choose a reason for hiding this comment

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

@ahrtr this is a good start but couple of comments:

  • You haven't covered a case when a major release and previous minor supported release? e.g. 4.0 and 3.x.x
  • One thing that I worry is that not every user will upgrade to newest patch release. e.g. users moving to 3.6 from 3.4.x or 3.5.x (when few more patch releases were available after 3.5.x)? To see what's changed they need to go find the patch releases of 3.5?
  • Also, users may miss some of the new changes that's going in main branch because main branch changelog was not updated as we backported those changes that also went into the main but we only updated changelog of backported release branch right?
    Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @spzala for the quick response.

  1. Yes, it's a good catch, just updated.
  2. Yes, users need to read all the changelogs between current version and the target version to which they plan to upgrade. We can's take care of all such scenarios.
  3. This isn't a problem. Let's work with three examples.
    • Example 1: Assuming a user adds a new feature to main, and backports to both 3.5 and 3.4, then we shouldn't update 3.6 changelog item, because 3.6.0 isn't released yet.
    • Example 2: Assuming 3.6.0 is already released, and release-3.6 branch is cut. We need to follow the first rule Each patch release only includes changes against previous patch release. to update the changelog-3.6. If a user adds a new feature to main, and backport to 3.5 and 3.6, then both 3.5 and 3.6 changelogs need to be updated.
    • Example 3: Assuming 3.6.0 is already released, but release-3.6 branch isn't cut yet. I don't think we will run into this case. Each time when we release the first major or minor version, a related branch must be cut.

Copy link
Member Author

Choose a reason for hiding this comment

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

one more comment for # 3, usually we don't backport features. But there is no hard limitation on this. I think the same thing to changelog item.

If all maintainers agree, we can also add changelog item for both main and previous version for some important feature.For example, @serathius added changelog item into both 3.6 and 3.5 for #14039

What do you think? @spzala

Copy link
Member

@spzala spzala Oct 11, 2022

Choose a reason for hiding this comment

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

@ahrtr thank you so much for the detailed answers. As you noticed, some cases if maintainers agree we will update both the patch release and main branch changelog. That's good but then there may be some items that's added to main changelog and some not, I am not sure if we (specially non developer users) can easily track it. I overall feel that keeping main branch changelog (as we used to) is more user-friendly. There will be some redundancy but users can get changes at one place over previous minor release. Also, I think of couple more scenarios:

  • There may be changes that may be only backported and not added in newer version. How user differentiate by looking at the changelog of a patch release of what's also changed in the main branch?
  • What happens when, as an example, v3.5.x3 and v3.5.x4 had changes that were only backported, where as v3.5.x2 has some changes that were also changed in the main branch. How easily users notice it?

The current way of updating main changelog gives one place to see what's changed over previous minor or major release (not keeping patch releases in account here). If we really want to change the current way, I am fine and see how it goes but also it would be good to get some more thoughts from you and others (including etcd users if we can). Thanks!
P.S. (new edit)
Also, from development perspective, as we know sometime backporting changes take long (and erroneously can be missed) or sometime someone comes out with the need of backporting long time after a PR is merged in the main branch. So in that case, do we first update the changelog of main branch and remove the entry from changelog when we backport (as we did with one or two PR)?. I think that's not quite efficient but probably okay.
More I think, considering we won't backport everything (specially features), I am okay with anyway we go forward. We just want to clearly point out few things for users. Thanks!

Copy link
Member Author

@ahrtr ahrtr Oct 11, 2022

Choose a reason for hiding this comment

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

Thanks @spzala for the valuable comment, which makes sense to me.

The current way of updating main changelog gives one place to see what's changed over previous minor or major release (not keeping patch releases in account here)

This seems correct and good to me. Specifically, the main changelog (first release of each minor or major version) should only include:

  • new features. (Even a feature is backported, we also need to add it into main changelog).
  • breaking changes
  • deprecations
  • Bumping of dependencies. Probably we only need to list the core dependencies bumping, such as Go, gRPC.etc. WDYT?
  • Other changes, which should be added per maintainers' discussion

One thing to be clearer, I don't think the main changelog shouldn't have bug fixes. Because a bug must be

  • either introduced in previous version and accordingly need to be backported
  • or introduced only in main branch, so no need to backport. In this case, we don't need to list either because it (e.g. 3.6.0) hasn't be released yet.

Copy link
Member Author

Choose a reason for hiding this comment

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

FYI. I just updated the second rule to reflect the discussion. Basically we are still following the existing rules.

Copy link
Member

@spzala spzala left a comment

Choose a reason for hiding this comment

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

@ahrtr thank you for working on it. I am good to give it at try. cc @serathius - if you have any comments. Thanks!

Signed-off-by: Benjamin Wang <wachao@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants