-
Notifications
You must be signed in to change notification settings - Fork 85
Add script to compile Changelog from individual entries #7177
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
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7177 +/- ##
==========================================
+ Coverage 87.17% 87.24% +0.06%
==========================================
Files 535 535
Lines 35330 35335 +5
Branches 4113 4114 +1
==========================================
+ Hits 30800 30828 +28
+ Misses 3639 3617 -22
+ Partials 891 890 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Greptile SummaryIntroduces a fragment-based changelog system to eliminate merge conflicts when multiple PRs edit
Issue Found: The code validates Confidence Score: 3/5
Important Files Changed
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
9 files reviewed, 4 comments
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
9 files reviewed, 1 comment
noxfiles/changelog_nox.py
Outdated
| if "pr" not in data: | ||
| missing.append("pr") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't see a check to verify the PR number etc is added before merging. Its not a huge issue, just kind of a gap. The change could be added to the changelog without a PR # or link.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe in the workflow file we could add something like this to catch any errors at PR time instead of release time.
- name: Validate changelog entries
run: |
pip install pyyaml
nox -s "changelog(dry)"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a nox -s "changelog(validate)" that allows specifying only specific files
JadeCara
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a comment on a potential gap, other than that looks good to me. The commands ran as expected!
Thanks for the excellent updates to the documentation!
Co-authored-by: Jason Gill <jason.gill@ethyca.com>
| - In the description and code changes sections, do not include information about code formatting or general code clean up changes. | ||
| - Do not include running tests as part of the Steps to Confirm, those will run automatically as part of the PR. | ||
| - The issue can usually be derived from the current branch name, it's a Jira ticket with prefix "ENG-" (eg. `Ticket [ENG-1234]`) | ||
| - If `acli` is available, use `acli jira workitem view [key]` cli command to get more information about the issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry, this one needs it also:
"Full permissions must be used for the acli credentials to work, even when in sandbox mode."
Ticket []
Description Of Changes
This PR proposes a new way to create our release Changelog. Right now, it's very common to add your entry to the
CHANGELOG.mdfile in a PR, and then have to constantly resolve conflicts on it as other PRs that also edited the file get merged. This PR proposes an entry-based approach, where each PR adds an entirely new file in a/changelogdirectory; the individual entries will get "compiled" into theCHANGELOG.mdfile as part of the release process via a nox script.I think there's two benefits to this approach:
CHANGELOG.mdfile.Code Changes
/changelogdirectory with aTEMPLATE.yamlfile that shows the format for the changelog entriesnox -s changelogsession (see more below)releases.mdto reflect this new processpr-command.mdso that agents know how to create changelog entriesUsage
nox -s "changelog(dry)"-> dry run of the command, will print to console the output that would be generated but will not make any changesnox -s "changelog(write)" -- --release 2.77.0-> will generate the changelog for the provided release version from the entries in the/changelogdirectory . This will also delete the entries from the/changelogdirectory.nox -s "changelog(write)" -- --release 2.76.2 --prs 1234,5678-> will only compile the entries for the given PRs into theCHANGELOG.mdfile, leaving the rest as-is. This is useful for patch releases where only some commits will get released.Pre-Merge Checklist
CHANGELOG.mdupdatedmaindowngrade()migration is correct and works