Some Thoughts on Spelling Fixes
I've been looking at a number of FreeBSD pull requests lately. Many of them are spelling fixes. I'd like to offer some suggestions for people submitting them. The same advice applies to typo fixes and grammar corrections.
- Keep number of changes small.
- Use separate commits per directory
- Use descriptive commit messages
- Set your email correctly
- Don't correct code
Keep number of changes small
When submitting like this, limit the number of changes to 10-20. More than about 15 changes becomes hard to review. Every single change has to be verified for correctness, and having too many will make your pull request more likely to be overlooked.
Use separate commits per directory
When submitting a number of changes, do one change per subdirectory. When subdirectories are nested, it's OK. For example, if you have changes to bin/ls and bin/rm, do two commits. But if you have changes to both usr.sbin/wpa/wpa_priv and usr.sbin/wpa/wpa_cli, then it's OK to do those as one commit.
Use descriptive commit messages
The commit message "fix spelling" is too generic to be useful. If you are fixing just one word, a better commit message would be "Spell interrupt correctly". It also allows the reviewer to make sure that you are changing the right thing. And it also gives enough detail that people skimming the logs don't need to look at the diffs to know what changed. If you are fixing multiple spelling errors, then a generic message is more appropriate.
Set your email correctly
When you push your branch to github, make sure that you've set the email you want in the commit message. It saves a lot of time. If you are using github's editor, make sure that your profile has this information set correctly.
Don't correct code
Don't make spelling corrections in code variables, #defines, etc. These will likely be ignored. The risk from a comment or an error message being corrected is tiny, while code changes could be attempts to subtly change the system or introduce security impacting issues through a supply chain attack. It's better to work with someone in the community to get these corrected than to correct them via a pull request.
> Use separate commits per directory
ReplyDeleteIMHO, it is bad criteria. Proper one is "Use separate commits per atomic change". Each public (!) commit must be (in ideal world) compilable & pass all tests. Sometimes it is good idea to commit changes to different directories separately, but sometimes they should go into one commit even if they is not one subtree. For example, change in library and all programs which use this library. There should not be separate commit to library which breaks all library clients and N commits to N clients after this. It should be one commit. IMHO.
I'd consider this a code change, which I believe should be handled in coordination with a committer. And if you are changing the API, you'll need backwards compatible symbols that are misspelled for a transition period.
ReplyDeleteWhen fixing spelling in error messages or comments, though, each change is complete in and of itself. lumping them by directory is a reasonable tradeoff between having to do each one separately, or doing too many at once which cause merging issues.