RFC: PR titles (conv-commits modification)

[pvinis]

Proposal

After my comment here I promised I will ues the regular conv commits until I make an RFC. Here is that RFC.

And here is my modified list of conventional commits for our PR titles:

• HOTFIX: If an urgent fix is required.
• feat: A new feature.
• fix: A bug fix.
• docs: Documentation only changes.
• perf: A code change that improves performance.
• test: Adding missing tests or correcting existing tests.
• format: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc, linting).
• revert: Revert something.
• update or upgrade: Update dependencies or pods or other related things.
• refactor: A code change that keeps functionality mostly the same.
• build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm).
• uxui: changes that affect how something looks or how the user experiences it.
• cleanup: When the main change is removing code, old or unused.
• other: catch-all, and once we have had a bunch of those, we can possibly make a new category.

Reasoning

It all started with me hating attaching the word chore on any of my work. I refused to use it, and I have only used it begrudgingly for some PRs after my promise, and on very very few PRs that I would actually characterize as chores.

I then decided to do an investigation a few months ago. I took 300 commits from eigen, and checked their type. In these 300 commits we had:

• 20 chore
• 7 enhancement
• 50 feat
• 85 fix
• 1 format
• 9 refactor
• rest were merge commits, or uncharacterized commits.

So my thoughts after this were:

• fixes and feats are a lot, but that makes sense. We add new stuff, and we fix stuff. But we probably can be better at characterizing these, so adding labels like uxui might make PR titles slightly more specific.
• chore is a bit of a catch-all, and I think we can do better. We can use cleanup for removing code, and update for updating dependencies. We can also use other for things that don't fit in any of the other categories, while valuing our work a bit more, and not just calling it a chore.

This new list was suggested here. The main differences are:

• this list should not be limited to this, but it should be fine for like 95% of commits/PRs.
• update/upgrade, they are needed, this work is not a chore.
• chore can go live somewhere else, where bad words go to die. I replaced it with other. Better word, less negative, same length. But we can allow chore if someone really found some work to be a chore. It should not be the catch-all though.
• uxui, I don't know how this is missing from other lists. This is needed.

Additional Context

Lengthy discussions on slack:

• https://artsy.slack.com/archives/C02NAJ2CGLW/p1652704713254419
• https://artsy.slack.com/archives/C2TQ4PT8R/p1648141250236779

How is this RFC resolved?

Change the peril check. Change the graphs generated by the velocity team to include the new labels.

[kathytafel]

I sort of love this because I hate chores, too. I assume it wouldn't break the charts or you wouldn't have proposed it.

[admbtlr]

I guess you already knew that I would vote against this 😄. I like the standard conventional commit options for two reasons:

1. They are simple and I don't have to think about them
2. They are user-centric: they characterise the work in terms that matter to the user, not to the engineer.

Once your code is committed, this user-centricity is the right way to think about it IMHO: "what does this bring to the user?" This makes it easier to use the git log in two different ways:

1. As a tool for managing the product, e.g. (but not limited to) automated changelogs. A changelog will normally have three sections: new features, fixes and upkeep/maintenance (or it will skip the third section).
2. To understand how much time you're spending on building new stuff ("feat"), how much on rework ("fix") and how much on maintenance and upkeep ("chore").

Conventional commits allow you to use 3-5 characters (or 6 if you include the colon) to indicate which of the three sections this work fits into. And then you have the rest of the title, plus the description, to provide more context for what you actually did.

Is it so hard to gift those 3-5 characters to users and automation? Because honestly, the user doesn't care whether you refactored, reformatted or cleaned up, and neither does a user-centric analysis of the log.

[lidimayra]

My personal thoughts on this: I'm not a fan of conventional commits. I think conventional commits prefixes use a precious space that sometimes forces us to sacrifice words that could make the title much more descriptive. That being said, I understand their importance when we're talking about automation tools, changelogs and metrics collection.

When it comes to actual meaning and readability (very useful when looking at the commits history), what I really care about is always after the colon, not before. So I don't feel like putting too many thoughts into an extensive list of prefixes. If I'm cleaning something up, updating dependencies or refactoring code, I rather use always the same prefix (like chore as a standard) than have to think about a customized list to check if my work fits there (and in case it doesn't, label it as other).

And as much as I don't like the conventional commits, if we have to use them, I rather be sure that the prefixes will always be as short as possible (3 ~ 5 characters). Using prefixes like refactor, for example, would already consume 8 characters from my title with something that, as a human, I really don't care about and it would prevent me to write the actual title in the way I want to write.

[damassi]

Agree with @admbtlr here - and its worth noting that conventional-commits are a standard. As per other things discussed recently, the further we deviate from standards the more troublesome our code, pipelines and other tools become to manage, because that forces us to stray from open source, the wider community, etc. There are lots of tools we can take advantage of to make the absolute best of our git commits (which are important atomic units regardless of personal opinion) and in the future we might want to implement.

[damassi]

To use a practical example as seen in various repos, when we make library releases, auto scans our conventional commits, via the conventional-commit changelog plugin, to auto-generate changelogs for us that are pinned to specific releases. Examples:

• https://github.com/artsy/palette/releases.
• https://github.com/artsy/palette-mobile/releases
• https://github.com/artsy/icons/releases
• https://github.com/artsy/fresnel/releases
• etc

This is a perfect example of standardized tooling in practice, which starts to break down when we begin adding idiosyncratic categories on a whim. And this release generator can be applied to every one of our repos (not just libraries), since we typically have concrete release git shas that hokusai points at, etc. It's just a matter of an inspired engineer spending a future friday to set this up.

[dzucconi]

The whole point of conventional commits is that they follow a spec. The spec is there so that they can be machine readable by existing tooling and there is a community understanding of what is there and how to apply it. There is a ton of tooling surrounding these conventions, that help author them and adhere to the spec. I do not feel the need to complicate things here.

[dzucconi]

forces us to sacrifice words that could make the title much more descriptive

There's always commit body if you want to add a longer description though. That's totally valid within the spec.

[lidimayra]

Yeah, I always use the commit body as well, still, I like to have my titles as descriptive as possible. And for that, if I sacrifice 3 to 5 characters, that's still fine, if we start to use much longer non-standard terms, then it might force me to hide stuff from the title that I fail to see the benefit in it.

[pvinis]

The whole point of conventional commits is that they follow a spec. The spec is there so that they can be machine readable by existing tooling and there is a community understanding of what is there and how to apply it. There is a ton of tooling surrounding these conventions, that help author them and adhere to the spec. I do not feel the need to complicate things here.

every single tool for conv commits has support for a custom list of words. my point here is that we can use all these tools, but we could do that while using a better list of words.

[damassi]

every single tool for conv commits has support for a custom list of words

That's not true, unfortunately. If every tool had the ability to provide a custom list of words, then the definition of a "spec" would break down (though of course, one can imagine clever aliasing here). Auto.rc, for example, doesn't provide the ability to change the keywords and point specifically at the conventional-commits spec in their docs.

Browsing around Github on some random day, we see the conventional commits standard everywhere; it makes sense, is concise, and gets the job done without needing to micromanage every aspect of the developer experience.

[pvinis]

I see it does https://github.com/intuit/auto/tree/main/plugins/conventional-commits#preset

any tool about convcommits that doesn't support custom words is truly not worth using.

[damassi]

I stand corrected! But my point still applies. There's a common language (the spec) that would require significant retooling for it to be shared across a broad array of services, which dilutes the power of said spec.

[pvinis]

after 2 weeks, i see 4 yeses and 4 noes. i would very strongly suggest this becomes a thing, but since I'm not there anymore to fight for this and also gain from it being adopted, ill close this.

if any of you wants to reopen this at any point, please please do so. its one of those things that is slightly hard to change because it goes off the beaten path, but its one of those things that is so much better. the beaten path is a bad one, this one is a very human-friendly one.

[damassi]

its one of those things that is slightly hard to change because it goes off the beaten path, but its one of those things that is so much better. the beaten path is a bad one

It's important to recognize, too, when there's a certain personal bias involved, or blind spots tending towards unilateralism of action. To use a similar but different standard, rejecting the commonest one of them all, React.FC types. 99.99% of React codebases have adopted this standard. It is the React standard. Why do we have to make right turns and reject this in our code, to the point where FC types are actually being removed in PRs? What problem are we solving here? What problems are we creating? Custom types, custom rules, more idiosyncratic code, single-developer dependencies? Personal projects, all good. But when working on a large team and at scale?

Swapping FC types are harmless, sure, but this goes for everything. What happens when one starts dealing with serious amounts of money and vastly more complexity, and the stakes are much (much) higher, with hackers focusing on your platform every hour of every day? It's good to be more circumspect, and to think big picture and to recognize that there's wisdom in the beaten path, just as there's wisdom in standards, however much ones nature screams that there's not, in the face of all evidence to the contrary. It's something to ponder moving into your new role.

[pvinis]

that's true. but when someone has gotten used to something, that's also a bias that can hold you back from a better thing.

(let's not talk about the FC thing because I was right about this way before even the react core team decided to go with what I suggested much earlier 😅)