react-spectrum
react-spectrum copied to clipboard
adobe
Update changelog
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
Build successful! 🎉
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!
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?
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.
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.
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
@PHILLIPS71 Sorry it took me so long to reply, kinda got caught up in other things but yeah, I think this is the kind of direction we'll want to go so thanks for the suggestion! After I worked on this, I realized it would probably be better to implement some sort of linter first for the PR titles haha