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

Switch to file-per-change changelog #3774

Merged
merged 19 commits into from
Aug 1, 2024

Conversation

ysaito1001
Copy link
Contributor

@ysaito1001 ysaito1001 commented Jul 25, 2024

Motivation and Context

Implements RFC: File-per-change changelog

Description

When this PR is merged to main, it will impact developer workflow for creating a changelog entry that goes with a PR.
Prior to this, one edited CHANGELOG.next.toml to enter a changelog entry, but it often caused merge conflicts among us. With this PR, one creates a .md file (any filename stem will do) under the smithy-rs/.changelog directory. This Markdown file has the YAML front matter, and it can be created manually or via the ChangeLogger CLI (see this commit for more details).

Here are the highlights of the code changes:

  • The release script has been pointed to smithy-rs/.changelog. This is pretty much what it takes to let the smithy-rs release infra switch to the new changelog mode.
  • sdk-lints now disallows CHANGELOG.next.toml. It will complain if the file is present.
  • The --source-to-truncate option for the changelogger's render is now optional because our internal release infra code no longer needs to specify that option (i.e. there is nothing to truncate there).
  • Tests in smithy-rs-tool-common and changelogger have been updated so TOML changelog entries used in tests have been re-written to Markdown changelog entries. We want to avoid giving the false impression that TOML changelog entries are still well-supported.

Testing

  • Existing tests & lints in CI
  • Ran smithy-rs dry-run release and confirmed the correctness of CHANGELOG.md and SDK_CHANGELOG.next.json in the release artifacts
  • Ran the internal release and confirmed CHANGELOG.md was rendered correctly in aws-sdk-rust

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@ysaito1001 ysaito1001 marked this pull request as ready for review July 25, 2024 21:06
@ysaito1001 ysaito1001 requested review from a team as code owners July 25, 2024 21:06
Copy link
Contributor

@landonxjames landonxjames left a comment

Choose a reason for hiding this comment

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

Nice! Look forward to not having to merge from main to fix the CHANGELOG ever again

CONTRIBUTING.md Outdated Show resolved Hide resolved
@smithy-lang smithy-lang deleted a comment from github-actions bot Jul 25, 2024
@smithy-lang smithy-lang deleted a comment from github-actions bot Jul 25, 2024
Copy link

A new generated diff is ready to view.

  • No codegen difference in the AWS SDK
  • No codegen difference in the Client Test
  • No codegen difference in the Server Test
  • No codegen difference in the Server Test Python
  • No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

Copy link

A new generated diff is ready to view.

  • No codegen difference in the AWS SDK
  • No codegen difference in the Client Test
  • No codegen difference in the Server Test
  • No codegen difference in the Server Test Python
  • No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

@drganjoo
Copy link
Contributor

Just curious, why is the .changelog a directory and not a file? Especially since the PR description mentions that a .md file will be created. What does the changelogger tool do if multiple files are present? Also, what is the recommendation if a PR closes multiple open tickets? Should we create multiple .md files or just one and add entries to the references field list?

tools/ci-build/changelogger/Cargo.toml Outdated Show resolved Hide resolved
tools/ci-build/changelogger/README.md Show resolved Hide resolved
tools/ci-build/changelogger/README.md Show resolved Hide resolved
tools/ci-build/changelogger/README.md Show resolved Hide resolved
tools/ci-build/changelogger/src/render.rs Show resolved Hide resolved
@drganjoo
Copy link
Contributor

Thank you for making this change to the changelogger!

Base automatically changed from ysaito/add-new-subcommands-to-changelogger to main July 31, 2024 22:01
Copy link

A new generated diff is ready to view.

  • No codegen difference in the AWS SDK
  • No codegen difference in the Client Test
  • No codegen difference in the Server Test
  • No codegen difference in the Server Test Python
  • No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

Copy link

github-actions bot commented Aug 1, 2024

A new generated diff is ready to view.

  • No codegen difference in the AWS SDK
  • No codegen difference in the Client Test
  • No codegen difference in the Server Test
  • No codegen difference in the Server Test Python
  • No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

@ysaito1001
Copy link
Contributor Author

Just curious, why is the .changelog a directory and not a file? Especially since the PR description mentions that a .md file will be created.

Not sure if I understand the question? If .changelog were a file, that'd be another version of CHANGELOG.next.toml where everyone would step on each other. The .changelog directory is a place to store individual changelog entry Markdown files by which we can avoid merge conflicts when we generate changelog entries (see RFC).

What does the changelogger tool do if multiple files are present?

As of this PR, while rendering release notes, channgelogger "folds" individual changelog entry Markdown files in the .changelog directory into what would otherwise be the contents of CHANGELOG.next.toml. But we don't have to care about that; we just need to create individual .md files under the .changelog directory.

Also, what is the recommendation if a PR closes multiple open tickets?

This won't change compared to what we've been doing for CHANGELOG.next.toml: list multiple issues in references.

Should we create multiple .md files or just one and add entries to the references field list?

Again, this is the same as today, just list multiple issues in references in a single Markdown file. If you use the changelogger CLI, then you can specify --references <issue#> (or -r <issue#>) repeatedly.

@ysaito1001 ysaito1001 added this pull request to the merge queue Aug 1, 2024
Merged via the queue into main with commit 10a9205 Aug 1, 2024
51 checks passed
@ysaito1001 ysaito1001 deleted the ysaito/switch-to-file-per-change-changelog branch August 1, 2024 04:59
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.

5 participants