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

[Release] Add release.yml for release notes automation #2732

Merged

Conversation

ZilongX
Copy link
Collaborator

@ZilongX ZilongX commented Nov 2, 2022

Signed-off-by: Zilong Xia zilongx@amazon.com

Description

Screen Shot 2022-11-02 at 10 41 44 AM

Issues Resolved

Resolved #2677

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
    • yarn test:ftr
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

Signed-off-by: Zilong Xia <zilongx@amazon.com>
@ZilongX ZilongX requested a review from a team as a code owner November 2, 2022 17:45
@ZilongX ZilongX added the release label Nov 2, 2022
@ashwin-pc
Copy link
Member

Hey @ZilongX, I've called out some concerns in the original issue about this approach. Since the initial changelog was introduced in a rush without proper discussion, I don't want us to slam another solution in without coming to a conclusion on that thread. There seem to be a lot of good ways to do it, and this might be one too, but let's settle on it before pushing this in.

@seraphjiang
Copy link
Member

Hey @ZilongX, I've called out some concerns in the original issue about this approach. Since the initial changelog was introduced in a rush without proper discussion, I don't want us to slam another solution in without coming to a conclusion on that thread. There seem to be a lot of good ways to do it, and this might be one too, but let's settle on it before pushing this in.

@ashwin-pc

Don't see any conflict to try. this approach has no additional conflict of workload added. it is very much doable, wait for last step to verify the gap.

It is two-ways door, we could revert if result is not we want.

@kavilla
Copy link
Member

kavilla commented Nov 2, 2022

Hey @ZilongX, I've called out some concerns in the original issue about this approach. Since the initial changelog was introduced in a rush without proper discussion, I don't want us to slam another solution in without coming to a conclusion on that thread. There seem to be a lot of good ways to do it, and this might be one too, but let's settle on it before pushing this in.

Tbh, the changelog is an organization decision

@ashwin-pc
Copy link
Member

Don't see any conflict to try. this approach has no additional conflict of workload added. it is very much doable, wait for last step to verify the gap.

@seraphjiang You are right, it is a 2 way door. But i disagree with the workload added. This flow only addresses release notes and not changelog, So even with this solution, we dont really close the gap for changelogs pain. Secondly, it relies on good PR titles and labels. Thats 67 PR's we need to label and edit for the generated notes to work. something i'm sure @AMoo-Miki might not be happy to add to his release checklist.

From your comment it looks like you want to use this as an experiment to test and see if this solves part of our release process pain, and for that i think we can get the answer to that from the POC @ZilongX has implemented in https://github.com/opensearch-project/opensearch-dashboards-functional-test. Its also has a much smaller blast radius and can give us a good idea of the pro's and cons of this approach.

Also given that we are at code freeze for 2.4. Adding this to the already messy changelog problem might cause more harm than good.

@ZilongX
Copy link
Collaborator Author

ZilongX commented Nov 22, 2022

@opensearch-project/opensearch-dashboards-core maintainers poke for a re-review following the discussion, let's proceed the auto generation of release notes (for experimentation for now) if no other concerns

@codecov-commenter
Copy link

codecov-commenter commented Nov 22, 2022

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.66%. Comparing base (14af23a) to head (dc3b6d7).
Report is 1 commits behind head on main.

Current head dc3b6d7 differs from pull request most recent head 2dc48aa

Please upload reports for the commit 2dc48aa to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2732      +/-   ##
==========================================
- Coverage   67.44%   66.66%   -0.78%     
==========================================
  Files        3442     3219     -223     
  Lines       67816    61448    -6368     
  Branches    11027     9417    -1610     
==========================================
- Hits        45740    40967    -4773     
+ Misses      19409    18234    -1175     
+ Partials     2667     2247     -420     
Flag Coverage Δ
Linux_1 ?
Linux_2 ?
Linux_3 ?
Linux_4 ?
Windows_1 ?
Windows_2 ?
Windows_3 ?
Windows_4 ?

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ashwin-pc
Copy link
Member

@ZilongX I've called out my reasons for holding off on this change for now. Let me know if you disagree with my reasons. My assumptions could be wrong, but my goal is to make sure we dont merge this in without a plan to solve our original pain point (changelog and release notes generation)

@ashwin-pc
Copy link
Member

Moving this to draft for now until.

@ashwin-pc ashwin-pc marked this pull request as draft November 30, 2022 01:52
@kavilla
Copy link
Member

kavilla commented May 8, 2023

Thanks for opening @ZilongX! It will seem we are moving forward with this proposal here: opensearch-project/.github#156.

If you have an feedback please comment there! Thanks!

@kavilla kavilla closed this May 8, 2023
@seraphjiang seraphjiang reopened this Jun 11, 2024
Copy link
Contributor

❌ Invalid Changelog Heading

The '## Changelog' heading in your PR description is either missing or malformed. Please make sure that your PR description includes a '## Changelog' heading with proper spelling, capitalization, spacing, and Markdown syntax.

Copy link
Member

@seraphjiang seraphjiang left a comment

Choose a reason for hiding this comment

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

Let's try the github build-in release note generate to see if we could expedite the process

@seraphjiang
Copy link
Member

let's get this in and try, no need to wait for test, since this is just config file for release note generator

@Flyingliuhub Flyingliuhub added the Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry label Jun 11, 2024
@Flyingliuhub Flyingliuhub merged commit 2c58c99 into opensearch-project:main Jun 11, 2024
58 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jun 11, 2024
Signed-off-by: Zilong Xia <zilongx@amazon.com>
Co-authored-by: Tao Liu <33105471+Flyingliuhub@users.noreply.github.com>
(cherry picked from commit 2c58c99)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Flyingliuhub added a commit that referenced this pull request Jun 11, 2024
(cherry picked from commit 2c58c99)

Signed-off-by: Zilong Xia <zilongx@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Tao Liu <33105471+Flyingliuhub@users.noreply.github.com>
@zhyuanqi zhyuanqi mentioned this pull request Jun 11, 2024
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x failed changeset release Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[PROPOSAL] Introduce in automatically generated release notes
7 participants