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

DOC/DEV: update commit message guidance #20739

Open
lucascolley opened this issue May 18, 2024 · 4 comments
Open

DOC/DEV: update commit message guidance #20739

lucascolley opened this issue May 18, 2024 · 4 comments
Labels
Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org DX Everything related to making the experience of working on SciPy more pleasant

Comments

@lucascolley
Copy link
Member

lucascolley commented May 18, 2024

This is a note-to-self to update http://scipy.github.io/devdocs/dev/contributor/development_workflow.html#writing-the-commit-message. A few points:

  • I believe that "If the commit is related to a ticket, indicate that with
    "See #XXXX", "See ticket XXXX", "Closes #XXXX", or similar" is actually an anti-pattern (at least in some opinions), as it can clutter the linked issue. 'Closes ...' belongs in the PR description, rather than in every commit, IMO. It should only really be included in the extended description if it significantly improves it.
  • It's useful to include the relevant module/function (when there is one) after the acronym, like ENH: stats.circmean: ... or MAINT: fft: .... This doesn't need to be enclosed in backticks.
  • The example given, ENH: add functionality X to SciPy.<submodule>., is not great, IMO. There is no need to include SciPy in the commit message - that's a given.

This may include updating the PR description template.

@github-actions github-actions bot added Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org DX Everything related to making the experience of working on SciPy more pleasant labels May 18, 2024
@ilayn
Copy link
Member

ilayn commented May 19, 2024

For item 2, spaces are not needed around colons since commit message is limited space so better spend it on making the commit message clearer instead. Hence MAINT:fft:Some commit messageis fine and makes grepping much easier.

@lucascolley
Copy link
Member Author

For item 2, spaces are not needed around colons since commit message is limited space so better spend it on making the commit message clearer instead. Hence MAINT:fft:Some commit messageis fine and makes grepping much easier.

Point taken, but most people do put spaces around the colons1, so I think it's probably not worth enforcing either way - maybe saying that either is okay (or including both in examples) makes sense.

Footnotes

  1. and I think it looks nicer...

@rgommers
Copy link
Member

Agree with points 2 and 3. One comment on 2" backticks are not necessarily bad, and useful especially when referring to names with underscores, because that prevents formatting in release notes from going wrong (PR titles are included, and underscores are special syntax in reST.

Point 1 is a bit more complex. I agree with the first part. However, if there's a single commit that fixes an issue and you don't force-push that a lot, then adding Closes gh-1234 in the commit message is helpful. Commit messages need to be descriptive, and for future debugging it's quite relevant to see what a commit fixed without having to figure out what PR it was included in. So this is more a "it depends".

@lucascolley
Copy link
Member Author

That all makes sense. This issue was partly motivated by the guidance to 'name and describe your PR as you would write a
commit message'. I think Closes gh-1234 is one case where it would be useful to draw a distinction between the two - "it depends" for commit messages, as you say, but almost never good for PR titles.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org DX Everything related to making the experience of working on SciPy more pleasant
Projects
None yet
Development

No branches or pull requests

3 participants