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

Add RFC for changing the smithy-rs changelog process #3313

Merged
merged 5 commits into from
Apr 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions design/src/SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
- [RFC-0039: Forward Compatible Errors](./rfcs/rfc0039_forward_compatible_errors.md)
- [RFC-0040: Behavior Versions](./rfcs/rfc0040_behavior_versions.md)
- [RFC-0041: Improve client error ergonomics](./rfcs/rfc0041_improve_client_error_ergonomics.md)
- [RFC-0042: File-per-change changelog](./rfcs/rfc0042_file_per_change_changelog.md)

- [Contributing](./contributing/overview.md)
- [Writing and debugging a low-level feature that relies on HTTP](./contributing/writing_and_debugging_a_low-level_feature_that_relies_on_HTTP.md)
1 change: 1 addition & 0 deletions design/src/rfcs/overview.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,4 @@
- [RFC-0039: Forward Compatible Errors](./rfc0039_forward_compatible_errors.md)
- [RFC-0040: Behavior Versions](./rfc0040_behavior_versions.md)
- [RFC-0041: Improve client error ergonomics](./rfc0041_improve_client_error_ergonomics.md)
- [RFC-0042: File-per-change changelog](./rfc0042_file_per_change_changelog.md)
133 changes: 133 additions & 0 deletions design/src/rfcs/rfc0042_file_per_change_changelog.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
RFC: File-per-change changelog
==============================

> Status: RFC
>
> Applies to: client and server

For a summarized list of proposed changes, see the [Changes Checklist] section.

Historically, the smithy-rs and AWS SDK for Rust's changelogs and release notes have been
generated from the `changelogger` tool in `tools/ci-build/changelogger`. This is a tool built
specifically for development and release of smithy-rs, and it requires developers to add
changelog entries to a root `CHANGELOG.next.toml` file. Upon release, the `[[smithy-rs]]` entries
in this file go into the smithy-rs release notes, and the `[[aws-sdk-rust]]` entries are associated
with a smithy-rs release commit hash, and added to the `aws/SDK_CHANGELOG.next.json` for
incorporation into the AWS SDK's changelog when it releases.

This system has gotten us far, but it has always made merging PRs into main more difficult
since the central `CHANGELOG.next.toml` file is almost always a merge conflict for two PRs
with changelog entries.

This RFC proposes a new approach to change logging that will remedy the merge conflict issue,
and explains how this can be done without disrupting the current release process.

The proposed developer experience
---------------------------------

There will be a `changelog/` directory in the smithy-rs root where
developers can add changelog entry Markdown files. Any file name can be picked
for these entries. Suggestions are the development branch name for the
change, or the PR number.

The changelog entry format will change to make it easier to duplicate entries
across both smithy-rs and aws-sdk-rust, a common use-case.

This new format will make use of Markdown front matter in the YAML format.
This change in format has a couple benefits:
- It's easier to write change entries in Markdown than in a TOML string.
- There's no way to escape special characters (such as quotes) in a TOML string,
so the text that can be a part of the message will be expanded.

While it would be preferable to use TOML for the front matter (and there are libraries
that support that), it will use YAML so that GitHub's Markdown renderer will recognize it.

A changelog entry file will look as follows:

```markdown
---
# Adding `aws-sdk-rust` here duplicates this entry into the SDK changelog.
applies_to: ["client", "server", "aws-sdk-rust"]
authors: ["author1", "author2"]
references: ["smithy-rs#1234", "aws-sdk-rust#1234"]
# The previous `meta` section is broken up into its constituents:
breaking: false
# This replaces "tada":
new_feature: false
bug_fix: false
---

Some message for the change.
```

Implementation
--------------

When a release is performed, the release script will generate the release notes,
update the `CHANGELOG.md` file, copy SDK changelog entries into the SDK,
and delete all the files in `changelog/`.

### SDK Entries

The SDK changelog entries currently end up in `aws/SDK_CHANGELOG.next.json`, and each entry
is given `age` and `since_commit` entries. The age is a number that starts at zero, and gets
incremented with every smithy-rs release. When it reaches a hardcoded threshold, that entry
is removed from `aws/SDK_CHANGELOG.next.json`. The SDK release process uses the `since_commit`
to determine which changelog entries go into the next SDK release's changelog.

The SDK release process doesn't write back to smithy-rs, and history has shown that it
can't since this leads to all sorts of release issues as PRs get merged into smithy-rs
while the release is in progress. Thus, this `age`/`since_commit` dichotomy needs to
stay in place.

The `aws/SDK_CHANGELOG.next.json` will stay in place in its current format without changes.
Its JSON format is capable of escaping characters in the message string, so it will be
compatible with the transition from TOML to Markdown with YAML front matter.

The `SDK_CHANGELOG.next.json` file has had merge conflicts in the past, but this only
happened when the release process wasn't followed correctly. If we're consistent with
our release process, it should never have conflicts.

### Safety requirements

Implementation will be tricky since it needs to be done without disrupting the existing
release process. The biggest area of risk is the SDK sync job that generates individual
commits in the aws-sdk-rust repo for each commit in the smithy-rs release. Fortunately,
the `changelogger` is invoked a single time at the very end of that process, and only
the latest `changelogger` version that is included in the build image. Thus, we can safely
refactor the `changelogger` tool so long as the command-line interface for it remains
backwards compatible. (We _could_ change the CLI interface as well, but it will
require synchronizing the smithy-rs changes with changes to the SDK release scripts.)

At a high level, these requirements must be observed to do this refactor safely:
- The CLI for the `changelogger render` subcommand _MUST_ stay the same, or have minimal
backwards compatible changes made to it.
- The `SDK_CHANGELOG.next.json` format can change, but _MUST_ remain a single JSON file.
If it is changed at all, the existing file _MUST_ be transitioned to the new format,
and a mechanism _MUST_ be in place for making sure it is the correct format after
merging with other PRs. It's probably better to leave this file alone though, or make
any changes to it backwards compatible.

Future Improvements
-------------------

After the initial migration, additional niceties could be added such as pulling authors
from git history rather than needing to explicitly state them (at least by default; there
should always be an option to override the author in case a maintainer adds a changelog
entry on behalf of a contributor).

Changes checklist
-----------------

- [ ] Refactor changelogger and smithy-rs-tool-common to separate the changelog
serialization format from the internal representation used for rendering and splitting.
- [ ] Implement deserialization for the new Markdown entry format
- [ ] Incorporate new format into the `changelogger render` subcommand
- [ ] Incorporate new format into the `changelogger split` subcommand
- [ ] Port existing `CHANGELOG.next.toml` to individual entries
- [ ] Update `sdk-lints` to fail if `CHANGELOG.next.toml` exists at all to avoid losing
changelog entries during merges.
- [ ] Dry-run test against the smithy-rs release process.
- [ ] Dry-run test against the SDK release process.

[Changes Checklist]: #changes-checklist
Loading