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

feat: Service client release notes #1780

Merged
merged 14 commits into from
Oct 4, 2024
Merged

Conversation

sichanyoo
Copy link
Contributor

@sichanyoo sichanyoo commented Sep 27, 2024

Issue #

1734

Description of changes

Current

Currently, the GitHub release notes that get generated with each new release only contains information about commits that got merged to main since last release. In addition, it aggregates all commits into release notes, even ones that are irrelevant to customers (such as chore commits or commits from automated user for updating models / endpoints).

Goal

We want to filter those commits out from the release notes, as well as include specific service client changes in the release notes.

Changes

In the build system, at any point in time, we have access to two JSON files that we can extract this info from.
This PR's changes use those two JSON files to generate enhanced release notes for GitHub releases.

The changes were tested by running an E2E test against dev stack.

*See comments on GH diff for more details.

New/existing dependencies impact assessment, if applicable

Conventional Commits

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link
Contributor Author

@sichanyoo sichanyoo left a comment

Choose a reason for hiding this comment

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

Comments for reviewers

@@ -0,0 +1,72 @@
//
Copy link
Contributor Author

@sichanyoo sichanyoo Sep 27, 2024

Choose a reason for hiding this comment

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

The containers for decoding info we need from the JSON files available to use from internal build system
&
The reader struct for changing file paths for tests (can't go ../ due to sandboxing limits)

@@ -18,21 +18,60 @@ struct ReleaseNotesBuilder {

// MARK: - Build

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Refactors how Strings are joined together to fix confusing behavior when we have omitted section(s). With this refactor, we'll always have each line in the release notes separated by a newline regardless of whether there's a omitted section or not.

The last line of release notes with link to full changelog needs manual \n to make sure we separate it from content from above; otherwise markdown renders it as part of previous section.

func buildCommits() -> String {
commits
.map { "* \($0)"}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add new sections for release notes; one for service feature changes & one for service documentation update changes.

@@ -45,6 +45,16 @@ class PrepareReleaseTests: CLITestCase {
createPackageVersion(previousVersion)
createNextPackageVersion(newVersion)

Copy link
Contributor Author

@sichanyoo sichanyoo Sep 27, 2024

Choose a reason for hiding this comment

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

Creates files with bare minimum strings for making this test pass bc ReleaseNotesBuilder now reads from these files assuming they exist in current directory.

@@ -0,0 +1,179 @@
//
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regression tests for ReleaseNotesBuilder.
If you're curious for what the new release notes content will look like, look at expected in test cases below.

Sichan Yoo added 3 commits September 27, 2024 16:19
…enarios the JSON files we need are located in parent directory of aws-sdk-swift, but due to sandboxing, we can't save the dummy files to parent directory and instead can only save it in current directory. Use the flag to differentiate the path to retrieve JSON files from.
@@ -12,7 +12,6 @@ class PackageManifestBuilderTests: XCTestCase {

let expected = """
<contents of prefix>

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this test expectation changed? This feature doesn't involve the manifest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test was failing; I don't think this test was being run as part of CI with the package refactor task from before.

@sichanyoo sichanyoo merged commit bfb7379 into main Oct 4, 2024
29 checks passed
@sichanyoo sichanyoo deleted the feat/service-release-notes branch October 4, 2024 20:45
sichanyoo added a commit that referenced this pull request Oct 4, 2024
sichanyoo added a commit that referenced this pull request Oct 4, 2024
jbelkins pushed a commit that referenced this pull request Oct 28, 2024
* Add features struct and feature struct for grabbing info we need from build request and feature to service name mapping JSON files.

* Add service feature & service documentation sections to release notes generation. Also, refactor it for consistent spacing between the lines even when sections are omitted.

* Update existing code that uses ReleaseNotesBuilder to reflect new changes.

* Update test that fails; possibly from previous package manifest change.

* Add regression tests for release notes builder.

* Only build service client sections in release notes if the repo is aws-sdk-swift.

* Add test flag (grr) to ReleaseNotesBuilder to allow tests. In real scenarios the JSON files we need are located in parent directory of aws-sdk-swift, but due to sandboxing, we can't save the dummy files to parent directory and instead can only save it in current directory. Use the flag to differentiate the path to retrieve JSON files from.

* Resolve Josh's PR comments

* Temporarily comment out repo has change check for running E2E test.

* Uncomment temporarily commented logic for generating empty manfiest if repo has no changes.

* Change SDK changes section name from AWS SDK for Swift to Miscellaneous & change commit filter criteria from discaring chore and Update, to including feat and fix.

---------

Co-authored-by: Sichan Yoo <chanyoo@amazon.com>
jbelkins pushed a commit that referenced this pull request Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants