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

Update changelog #6221

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Update changelog #6221

wants to merge 5 commits into from

Conversation

yihuiliao
Copy link
Collaborator

Currently, our team doesn’t have a defined PR naming convention, which generally suits our workflow until we have sort through all the commits for release notes. To make the process easier, it would be useful to start following some sort of naming convention so that we can easily categorize the PR’s into their respective categories, thus, reducing the manual labor of release notes.

Using the firefox-ios wiki as inspiration, they outline their expectations for what a PR name should look like. What stands out to me is that they clearly define a keyword to be used at the beginning of their PR’s to indicate what change was made. We actually do this already to some degree, but we’ve never formally defined what keywords are associated with what changes/category which I'd like to start doing.

However, unlike firefox-ios, I don’t think we have to be strict with the anatomy of where the keywords are placed (i.e. at the very beginning of the PR). This might be ideal, but since we’ve never had any sort of precedence of naming our PR’s in a particular way, I feel that just introducing certain keywords (wherever they might be placed) will be easier to adopt.

I’ve already gone through our previous release notes to see what keywords we use the most and used them to help categorize our PR’s. The table below defines which keyword is associated with which category in our release notes. The only real new keyword would be "pre-release" for the PR’s in the "Under Construction" category. Folks will just have to keep this in mind if they are working on pre-release components. "Update" might also be considered too broad and maybe a bit subjective but based on previous release notes, it generally has gone in the "Fixes" category.

Feel free to comment on the keywords and their respective categories. I’m hoping this encapsulates most of our PR’s but always open to other opinions and ideas.

Keyword Meaning Category (in release notes)
Add/Support/Feat(ure) Creating a capability Enhancements
Remove Removing a capability Fixes
Update A change to existing code Fixes
Fix Fixing a bug Fixes
Bump Increase the version of something Fixes
Docs A change to documentation only Docs
Pre-release In a pre-release state Under Construction
Revert Reverting a previous commit None

Examples:

Enhancements:

  • Add focus/blur events support for CheckboxGroup
  • Feat: Render children in DropIndicators
  • Support for Avatar in ListBox, Picker, ComboBox, and SearchAutocomplete

Fixes:

  • Fix submenu safe area edge case
  • Update nested drop regions to correctly focus child drop target
  • Bump lighting css
  • Remove gap shim

Docs:

  • Docs: fix useToast example
  • Fixing two broken links in docs

Under construction:

  • Pre-release: RAC Tree

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

🧢 Your Project:

RSP

@yihuiliao yihuiliao marked this pull request as ready for review April 17, 2024 20:14
Copy link
Member

@snowystinger snowystinger left a comment

Choose a reason for hiding this comment

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

I like the idea, have you looked into commit lint tools to help enforce these terms?
https://github.com/conventional-changelog/commitlint
https://dev.to/mahmudulhsn/install-husky-in-your-project-for-proper-commit-lint-with-pre-commit-hooks-25b2
or something a little more all encompassing like https://github.com/changesets/changesets

what were your thoughts?

@rspbot
Copy link

rspbot commented Apr 17, 2024

@adobe adobe deleted a comment from rspbot Apr 17, 2024
@yihuiliao
Copy link
Collaborator Author

I like the idea, have you looked into commit lint tools to help enforce these terms?

i’ve thought about commit lint but ultimately decided that i didn’t want to be that restrictive with commit messages. that said, maybe it’s not that big of a deal? it might be worth at least setting it up to get a feel for it. also i’m not sure if i could enforce these keywords in the pr name with the commit lint since you can always edit it on your own after.

something a little more all encompassing like https://github.com/changesets/changesets

as for changesets, i haven’t heard of it before so i’d like to spend more time researching it but seems interesting!

@snowystinger
Copy link
Member

I like the idea, have you looked into commit lint tools to help enforce these terms?

i’ve thought about commit lint but ultimately decided that i didn’t want to be that restrictive with commit messages. that said, maybe it’s not that big of a deal? it might be worth at least setting it up to get a feel for it. also i’m not sure if i could enforce these keywords in the pr name with the commit lint since you can always edit it on your own after.

They can edit the name of the PR, but they can't edit the name of the commit, which is what we build the change log on?

@yihuiliao
Copy link
Collaborator Author

yihuiliao commented Apr 18, 2024

They can edit the name of the PR, but they can't edit the name of the commit, which is what we build the change log on?

yeah but since i believe we employ the squash & merge policy, all our pr’s end up just being a single commit on main which i believe defaults to the PR title right now (but im not entirely sure what are settings are for this). so even if we linted the individual commits, they wouldn’t necessarily be what’s used as the commit message on main.

@snowystinger
Copy link
Member

They can edit the name of the PR, but they can't edit the name of the commit, which is what we build the change log on?

yeah but since i believe we employ the squash & merge policy, all our pr’s end up just being a single commit on main which i believe defaults to the PR title right now (but im not entirely sure what are settings are for this). so even if we linted the individual commits, they wouldn’t necessarily be what’s used as the commit message on main.

Ah, yes, good point. I think the commit on main becomes the name of all commits though. So we delete all but the first commit message typically (which is also usually the title). Then, if the first commit is gibberish or not useful, we replace it with the title.

Since our team is in charge of it and if we enforce good commit messages, I think we can simplify some of this manual process. Could try changing the settings for that as well. Lemme know in slack if you want me to try pulling some of that up. Or you can try it out in your own private repo under your user so you can make a recommendation here.

@PHILLIPS71
Copy link
Contributor

I've encountered similar issues in the past, using something like below to enforce semantic PR titles while using the Squash & Merge strategy made it a lot easier.

https://github.com/amannn/action-semantic-pull-request

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.

None yet

4 participants