cppalliance / mrdocs

MrDocs: A Clang/LLVM tool for building reference documentation from C++ code and javadoc comments.
https://mrdocs.com
Other
75 stars 16 forks source link

fix: bug reporters refer to Mrdocs repo #572

Closed fpelliccioni closed 4 months ago

fpelliccioni commented 4 months ago

Fixes https://github.com/cppalliance/mrdocs/issues/457

alandefreitas commented 4 months ago

Nice. Is it good to go?

alandefreitas commented 4 months ago

Thanks for looking at this issue. I'm suggesting a minor adjustment to the commit message format: Our project:

(By the way, I'm not sure if it should be "bug reporters" or "bug reporter".)

This message format maintains consistency and clarity across our commit history:

Both things make it easier for other team members and users to grasp the purpose of the change. It's a minor adjustment that improves readability and makes the codebase more accessible and easier to maintain. This information could even go in our Contributor Guidelines. Could you update the commit message accordingly and force push to your branch?

fpelliccioni commented 4 months ago

Thanks for looking at this issue. I'm suggesting a minor adjustment to the commit message format: Our project:

  • (i) Uses conventional commit messages. For instance, "fix: redirect bug reports from LLVM to MrDocs repository" aligns with this convention.
  • (ii) Start messages with a noun rather than a verb. For instance, "fix: bug reporters are referred to Mrdocs repo" aligns with this convention. As a third guideline, we should also
  • (iii) Avoid the passive voice when possible, so the example above would become "fix: bug reporters refer to Mrdocs repo."

(By the way, I'm not sure if it should be "bug reporters" or "bug reporter".)

This message format maintains consistency and clarity across our commit history:

  • (i) Conventional commits ensure our commit messages are descriptive and easy to understand for everyone browsing through commit logs. It allows anyone browsing the logs to filter the commit by category and CI to generate the automatic changelog.
  • (ii) Starting with a noun highlights the focus of the commit instead of the previous state of the codebase. And
  • (iii) The Passive voice tends to make sentences unclear or indirect. While it is often used to convey formality in Latin languages like Portuguese and Spanish, it does not have the same effect in other languages such as English and German.

Both things make it easier for other team members and users to grasp the purpose of the change. It's a minor adjustment that improves readability and makes the codebase more accessible and easier to maintain. This information could even go in our Contributor Guidelines. Could you update the commit message accordingly and force push to your branch?

Thanks a lot for your detailed feedback on the commit message format. I’ve updated the PR title to align with the conventions you mentioned. I appreciate the guidance on maintaining consistency and clarity in our commit history.

fpelliccioni commented 4 months ago

Nice. Is it good to go?

@alandefreitas : Yes

alandefreitas commented 4 months ago

You just changed the PR title. The commit message is still the same.

fpelliccioni commented 4 months ago

You just changed the PR title. The commit message is still the same.

I've now updated the commit message as well. Initially, I didn't think it was necessary because I assumed the squashed merge would make the commit message irrelevant. However, I understand that keeping it consistent is better for future reference if someone checks the PR.

alandefreitas commented 4 months ago

I've now updated the commit message as well.

Great. Thanks.

Initially, I didn't think it was necessary because I assumed the squashed merge would make the commit message irrelevant.

You're correct. I usually adjust the messages before merging the PR. The reason why it's good that the PRs come already in the right format is because other people will rarely be as careful, and even I sometimes forget to adjust them. You can see examples of that in the commit history.

However, I understand that keeping it consistent is better for future reference if someone checks the PR.

Thanks. I know our conversation got a little ChatGPTish up there, but I had to go through the usual rationale for each of these guidelines, and that's not a bad way to do it. 😂 In any case, thanks again.

I saw you picked up a lot of issues from the MVP and that's also great. Thanks again.

We need to prioritize that problem introduced in CI by #545 without reverting it. Otherwise, bugs from newer PRs will go undetected, stack on each other, and make it difficult to untangle.