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

Reorganize changelog entries for 5.1 #5131

Conversation

Amxx
Copy link
Collaborator

@Amxx Amxx commented Jul 30, 2024

The changelog entry that was automatically generated in #5124 is messy. This proposes to reorganise it following what we did for 5.0

@Amxx Amxx added this to the 5.1 milestone Jul 30, 2024
@Amxx Amxx requested review from ernestognw and cairoeth July 30, 2024 14:13
@Amxx Amxx modified the milestones: 5.1, 5.1-after-freeze Jul 30, 2024
@ernestognw
Copy link
Member

As discussed in our call, we should consider whether we want to use this format in later releases. For now it's looking good but I need to take a look to the tests since the gas estimation is failing (not a big deal)

Copy link
Member

@ernestognw ernestognw left a comment

Choose a reason for hiding this comment

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

I consider that an ordered changelog is more readable, but I also think there should be a consistent format. We've given priority to the breaking changes in the past (listing them at first) and for 5.0 each category is a folder.

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
Copy link
Member

@ernestognw ernestognw left a comment

Choose a reason for hiding this comment

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

I went ahead and pushed the suggested changes, I think this way the diff reads way better and we keep consistent order.

Last time the gas comparison failed but I'm guessing is because there's no report to compare against. Should be fine since we're not changing code in this PR

@ernestognw ernestognw changed the title Reorder/Reorganise changelog entry for v5.1 Reorganize changelog entries for 5.1 Sep 5, 2024
@ernestognw
Copy link
Member

We'll wait before cherry-picking everything from #5139, otherwise the changelog will regenerate

@github-actions github-actions bot force-pushed the changeset-release/release-v5.1 branch from aba9ff6 to 878a26c Compare October 2, 2024 20:18
@Amxx Amxx force-pushed the changeset-release/release-v5.1-reorder branch from 1165d61 to e740579 Compare October 2, 2024 20:36
@Amxx Amxx requested a review from ernestognw October 2, 2024 20:43
Copy link
Member

@ernestognw ernestognw left a comment

Choose a reason for hiding this comment

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

CI will likely fail given there's nothing to compare gas against (in compareGasReports.js). LGTM

@Amxx Amxx merged commit 2369a64 into OpenZeppelin:changeset-release/release-v5.1 Oct 2, 2024
13 of 15 checks passed
@Amxx Amxx deleted the changeset-release/release-v5.1-reorder branch October 2, 2024 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants