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

Suggest the usage of conventional commits as a standard #328

Open
RobPasMue opened this issue Jul 13, 2024 · 14 comments
Open

Suggest the usage of conventional commits as a standard #328

RobPasMue opened this issue Jul 13, 2024 · 14 comments
Labels
new-content New feature or request scipy-24

Comments

@RobPasMue
Copy link
Contributor

It is a typical use case to follow the Conventional commits standard for commiting

https://www.conventionalcommits.org/en/v1.0.0/

I'd suggest adding it as a best practice. Conventional commits allow users and maintainers identify what is the purpose of both commits and PRs. There are tools that allow enforcing it on repos (using pre-commit hooks for example) which also reduces the burden on the maintainer to review that users are following the guidelines

@RobPasMue
Copy link
Contributor Author

This issue might live better in a different repo? Just unsure which is the best location

@kierisi
Copy link
Contributor

kierisi commented Jul 16, 2024

@all-contributors please add @RobPasMue for code, review

Copy link
Contributor

@kierisi

I've put up a pull request to add @RobPasMue! 🎉

@RobPasMue
Copy link
Contributor Author

Thanks @kierisi!

@willingc
Copy link
Collaborator

willingc commented Aug 7, 2024

Hmm... @RobPasMue I think it is reasonable to suggest for adoption when there are more than a handful of contributors to a project. Even for a smaller project, it is useful if it is lightweight (i.e. only requiring the title format). If you want to try it out on pyOpenSci/pyosmeta repo, I think that would be great.

@willingc willingc added enhancement-feature something new to add to our guide new-content New feature or request and removed enhancement-feature something new to add to our guide labels Aug 7, 2024
@RobPasMue
Copy link
Contributor Author

Sure! I can give it a try there! :) thanks @willingc - I will try to work on this in the next week(s)

@Midnighter
Copy link
Contributor

I'm using this format for all of my commit messages (unless I get very tired or frustrated and know that I will squash commits anyway). So big 👍🏼 from me.

@lwasser
Copy link
Member

lwasser commented Aug 29, 2024

y'all - @RobPasMue mentioned this to me at the pyOpenSci scipy sprint as well. I'm interested in doing this too. i see @tkoyama010 uses them. what i don't understand is how it works for content.

so i see there is feat for conventional comments. for a package that is clear how to use it. But what is i'm adding a new tutorial.

do i use

add: new packaging tutorial on X
feat(tutorial): new packaging tutorial on X

or some other approach?
I really love that idea, especially for core staff to do it at pyOpenSci. And it could be optional to have a "way we do things" but it doesn't have to be strictly enforced. I just don't fully understand how to use it in a non-packaged development environment.

@willingc
Copy link
Collaborator

@lwasser Historically, conventional commits were used to add consistency to commit messages in large projects to aid in autogeneration of changelogs. By using conventional commits, a tool could parse the messages into groups of "related" commits.

Standards add cognitive load for contributors, esp. new contributors. By suggesting as a good practice, it calls attention that it's important to consider how we communicate a PR's change.

There are multiple ways that you could encode your example:

  • feat: add packaging tutorial on X
  • feat(tutorial): add packaging tutorial on X
  • docs: add packaging tutorial on X
  • docs(tutorial): add packaging tutorial on X
  • docs(X): add packaging tutorial on X

Personally, I think encouraging conventional commits helps add consistency. However, I wouldn't run a lint on conventional commits that would fail a PR though it would be fine to report the result of the lint. Keeping the cognitive load low and the project friendly helps to encourage new contributors who will over time adjust to the general norm of the project 😄

@tkoyama010
Copy link
Member

@all-contributors please add @RobPasMue for idea

Copy link
Contributor

@tkoyama010

I've put up a pull request to add @RobPasMue! 🎉

@tkoyama010
Copy link
Member

I'm basically a supporter of Conventional commits, but it would be fair to add gitmoji, which is used in projects such as the FastAPI project in my mind :)

@RobPasMue
Copy link
Contributor Author

I completely agree with many of @willingc's points! Thanks for bringing them up.

In many of our projects we use it in combination with https://github.com/twisted/towncrier to autogenerate changelogs based on the changes implemented.

The problem I see with only suggesting it as good practice is that, in that case, many contributors would not take it into consideration, causing confusion consequently... Example: a new comer sees that it is a "good practice" but he/she also sees that many people are not following it so he/she doesn't know how to proceed... and would probably just not follow it because of the cognitive load it implies.

In my experience, I know this is a big dilemma. And I get that enforcing it would imply some work on everybody's side. One of the things we tend to do in the projects I work on is to enforce conventional commits on PR titles. That is something easy to modify and check by a user. We even implemented an action that takes care of it for us (see https://actions.docs.ansys.com/version/stable/style-actions/index.html#commit-style-action). It just checks that the PR title is following conventional commits and, if it isn't, it fails that stage. We could combine it with some better error message so that the user is self-sufficient and knows how to modify it as well.

Then, something we also do (but this is a maintainer's decision) is to only allow for "Squash and merge", as well as defaulting to the PR title. That way the entire PR is referenced via a single conventional commit on the main branch - and users don't have to follow strictly conventional commits on every single commit.

Anyway, I'm happy not to enforce it too! Just putting it as a good practice is a great advantage for everybody. I'm just proposing some easy, low-maintenance options to actually follow the standard.

@tkoyama010
Copy link
Member

Then, something we also do (but this is a maintainer's decision) is to only allow for "Squash and merge"

+100 same with us in our project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-content New feature or request scipy-24
Projects
None yet
Development

No branches or pull requests

6 participants