Skip to content

docs: update pull request template#5591

Merged
jmayclin merged 3 commits intoaws:mainfrom
jmayclin:2025-10-16-pr-template
Nov 4, 2025
Merged

docs: update pull request template#5591
jmayclin merged 3 commits intoaws:mainfrom
jmayclin:2025-10-16-pr-template

Conversation

@jmayclin
Copy link
Copy Markdown
Contributor

Goal

Simplify the PR template

Why

Our current PR template includes lots of "developer documentation" which is not applicable to most PRs. Additionally, the most important item to document for a PR is why it is being done. This new PR format forces contributors to specify that.

How

We delete the old template and add a new one

Note that this might require some changes to the release note script

Testing

None

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


<!-- for significant features includes a release summary -->
<!-- The release summary must be a single line that starts with "release summary" -->
<!-- release summary: s2n-tls customers can now frobincate the wobble -->
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
<!-- release summary: s2n-tls customers can now frobincate the wobble -->
<!-- release summary: s2n-tls users can now dance the tango -->

Are we going to get tired of this joke if we see it on every PR xD?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd like to keep the release summary on the top :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hmm, @CarolYeh910 out of the last 10 PR's we merged in, only 2 of them had a release summary. And then another 2 kept the release summary blank but didn't actually remove the section 😆 .

Since it's ignored in the majority of PRs, that's why I voted to move it lower. Thoughts?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Release summary is usually needed for feature PRs, thus the sample of last 10 PRs could be biased 😉 If it's present, I kinda want to move it higher. On the other hand, I can imagine the content of release summary is overlapped with the goal, so I'm fine with either way :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I wonder if the solution is somewhere in the middle? Like we include <A> in the PR title to indicate that it's
something we want to "announce"? And then we could just populate the release notes from the goal?

feat(s2n-tls): <A> allow pure PQ security policies

Or something like that 🤷

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Interesting idea! Hopefully it works with Doug's release automation script 😆

jmayclin and others added 2 commits October 30, 2025 17:21
Co-authored-by: maddeleine <59030281+maddeleine@users.noreply.github.com>
@jmayclin jmayclin requested a review from maddeleine October 31, 2025 00:33
@jmayclin jmayclin added this pull request to the merge queue Nov 4, 2025
Merged via the queue into aws:main with commit 482af4b Nov 4, 2025
50 of 51 checks passed
@jmayclin jmayclin deleted the 2025-10-16-pr-template branch November 4, 2025 00:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants