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

Adopt new changelog standard #1821

Closed
wants to merge 5 commits into from

Conversation

peternied
Copy link
Member

Description

Added new human readable changelog for 2.0.0-rc1 and including
unreleased commits.

The release process we've been using for release-notes has not been
producing useful results, there has been discussion in
opensearch-project/OpenSearch#1868 to come up
with a practice. This is an attempt to start following the suggestion
and come up with feedback for the OpenSearch organization.

Signed-off-by: Peter Nied petern@amazon.com

Issues Resolved

Check List

  • New functionality includes testing
  • New functionality has been documented
  • Commits are signed per the DCO using --signoff

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.

Added new human readable changelog for 2.0.0-rc1 and including
unreleased commits.

The release process we've been using for release-notes has not been
producing useful results, there has been discussion in
opensearch-project/OpenSearch#1868 to come up
with a practice.  This is an attempt to start following the suggestion
and come up with feedback for the OpenSearch organization.

Signed-off-by: Peter Nied <petern@amazon.com>
Signed-off-by: Peter Nied <petern@amazon.com>
@@ -0,0 +1,62 @@
# Changelog
Copy link
Member Author

Choose a reason for hiding this comment

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

Recommendation use Rich Diff to see this file with the markdown rendered.

CHANGELOG.md Outdated
* Security patch for `org.apache.kafka:kafka-clients`
* Security patch for `org.springframework.kafka:spring-kafka-test`

## [2.0.0-rc1] - 2022-04-20
Copy link
Member Author

Choose a reason for hiding this comment

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

This section is comparable to https://github.com/opensearch-project/security/blob/main/release-notes/opensearch-security.release-notes-2.0.0.0-rc1.md#2022-04-20-version-2000-rc1

Note; I've collapsed/omitted several updates by maintainers, it might be worthwhile discussion why we should/shouldn't include some of those items.

Signed-off-by: Peter Nied <petern@amazon.com>
@codecov-commenter
Copy link

codecov-commenter commented May 3, 2022

Codecov Report

Merging #1821 (400f804) into main (b514a4a) will increase coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               main    #1821   +/-   ##
=========================================
  Coverage     60.80%   60.81%           
- Complexity     3186     3187    +1     
=========================================
  Files           253      253           
  Lines         17931    17943   +12     
  Branches       3204     3205    +1     
=========================================
+ Hits          10903    10912    +9     
  Misses         5451     5451           
- Partials       1577     1580    +3     
Impacted Files Coverage Δ
...iance/ComplianceIndexingOperationListenerImpl.java 60.86% <0.00%> (-1.45%) ⬇️
...a/org/opensearch/security/tools/SecurityAdmin.java 37.50% <0.00%> (-0.25%) ⬇️
...ensearch/security/ssl/DefaultSecurityKeyStore.java 67.63% <0.00%> (-0.03%) ⬇️
...ecurity/configuration/ConfigurationRepository.java 74.17% <0.00%> (+2.19%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b514a4a...400f804. Read the comment docs.

Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

  1. All the changes need a link to the PR that made that change.
  2. I would develop this as a mechanism for all plugins and core and release in unison, wouldn't want just security plugin to do it.

Copy link
Contributor

@rursprung rursprung left a comment

Choose a reason for hiding this comment

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

CHANGELOG.md Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
MAINTAINERS.md Outdated Show resolved Hide resolved
@peternied
Copy link
Member Author

I would develop this as a mechanism for all plugins and core and release in unison, wouldn't want just security plugin to do it.

Very true @dblock; however, that source issue is very much abstract. For the sake generating feedback with a digestible example I started this effort. The 'practice' documentation should move into the meta repository for ratification - did you have thoughts on staging this differently that is being done in this PR?

Signed-off-by: Peter Nied <petern@amazon.com>
@peternied
Copy link
Member Author

Per your comments @dblock

  1. All the changes need a link to the PR that made that change.

Done.

  1. I would develop this as a mechanism for all plugins and core and release in unison, wouldn't want just security plugin to do it.

My thoughts were first by making this change it could help answer the question 'What does this look like' and create a focal point for discussion. It sounds like you would also want a check on pull requests that would make sure this process was not overlooked - what do you think about making a separate issue in opensearch-project/OpenSearch to track this?

@peternied peternied marked this pull request as ready for review May 10, 2022 21:16
@peternied peternied requested a review from a team May 10, 2022 21:16
Signed-off-by: Peter Nied <petern@amazon.com>
Copy link
Contributor

@rursprung rursprung left a comment

Choose a reason for hiding this comment

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

this looks good, thanks!

CHANGELOG.md Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
@cliu123
Copy link
Member

cliu123 commented May 18, 2022

There've been the release drafter to generate draft release notes capturing all merged PRs. Why is this changelog needed?

@peternied
Copy link
Member Author

There've been the release drafter to generate draft release notes capturing all merged PRs. Why is this changelog needed?

@cliu123 This changelog isn't machine generated, its human generated. I believe that offers a huge leap in readability. What do you think of the utility of this changelog vs the git log generated release notes?

@cliu123
Copy link
Member

cliu123 commented May 18, 2022

This is a good idea that makes changelog available before release! It'd be perfect if it's done automatially instead of manually. It'd be very easy to forget to update CHANGELOG.md on each PR especially for new contributors who are not familiar with our develpment process. In comparison, the release drafter run in CI automatically documents the changes, which makes more sense.
Additionally, opensearch-project/OpenSearch#1868 is still under discussion. Could we wait for more feedback and a decision?

@peternied
Copy link
Member Author

Being that there is still some contention with enforcement, I'm going to close this PR. When the time comes that we can broach this topic I'll see about reopening. Thanks everyone for your consideration.

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.

None yet

6 participants