sparckles / Robyn

Robyn is a Super Fast Async Python Web Framework with a Rust runtime.
https://robyn.tech/
BSD 2-Clause "Simplified" License
3.9k stars 198 forks source link

ci(commitizen): enforce conventional-commit syntax #776

Closed iiian closed 2 months ago

iiian commented 3 months ago

Description

Hello world! :smile:

This PR addresses #703, at least on a git-client-side. If a user runs the regular git commit -m "..." flow, they should now be prompted to use conventional commit syntax.

However, since git-users are always allowed to git commit --no-verif and skip commit hooks, let's discuss if you guys want this to run at a CI level as well.

vercel[bot] commented 3 months ago

@iiian is attempting to deploy a commit to the sparckles Team on Vercel.

A member of the Team first needs to authorize it.

iiian commented 3 months ago

Okay, so it seems to be working on my fork. https://github.com/iiian/Robyn/pull/1

The run from a couple of moments ago with a bad title: https://github.com/iiian/Robyn/actions/runs/8574622390

The run from just now with a good title: https://github.com/iiian/Robyn/actions/runs/8574757710

sansyrox commented 3 months ago

Nice! @iiian , one more question - does it enforce only on pr titles? or on commit messages too?

codspeed-hq[bot] commented 3 months ago

CodSpeed Performance Report

Merging #776 will not alter performance

Comparing iiian:ci/commitizen (a9a545d) with main (d95eb84)

Summary

✅ 108 untouched benchmarks

iiian commented 3 months ago

Nice! @iiian , one more question - does it enforce only on pr titles? or on commit messages too?

Yeah! Well, so I'm not totally convinced that there's anything enforcing that needs to happen in the body per se. Like, if conventional commit syntax is

<type>[optional scope]: <description>

[optional body]

[optional footer(s)]

optional footers are automatically the last contiguous block of lines after a double-new-line. Maybe its arguable that BREAKING CHANGES appearing inline in the body is a lintable offense because it smells like a possible mistake? Was there anything in particular you were hoping to lint the body for, like are you using any of the conventional commit footers in particular or auto-generating a changelog? It might also help if I went into the pull_request_template.yml and updated it to be a commitzen-like template:

# file: pull_request_template.yml
<!-- 
Please refer to the commitizen documentation if you are unfamiliar with commitizen. Commitizen formalizes git commit messages for the sake of automation and version management. 

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

Additionally, please build and test your changes before submitting a PR. Thanks :)
-->
<!-- STEPS -->
<!-- 0) Make your PR title the conventional commit type/scope/description
<!-- 1) Optionally, insert a body paragraph here describing the nature of your commit. -->
This chunk of work introduces awesome bells and whistles that are super pleasing to the human ear.

<!-- 2a) Does this PR address a particular issue? List them here. -->
Fixes: #104, #105
<!-- 2b) Does this PR introduce breaking changes? Include the following flag, with an optional description -->
BREAKING CHANGES[: optional description]
vercel[bot] commented 2 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
robyn ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 11, 2024 5:07am
sansyrox commented 2 months ago

Nice! @iiian , one more question - does it enforce only on pr titles? or on commit messages too?

Yeah! Well, so I'm not totally convinced that there's anything enforcing that needs to happen in the body per se. Like, if conventional commit syntax is

<type>[optional scope]: <description>

[optional body]

[optional footer(s)]

optional footers are automatically the last contiguous block of lines after a double-new-line. Maybe its arguable that BREAKING CHANGES appearing inline in the body is a lintable offense because it smells like a possible mistake? Was there anything in particular you were hoping to lint the body for, like are you using any of the conventional commit footers in particular or auto-generating a changelog? It might also help if I went into the pull_request_template.yml and updated it to be a commitzen-like template:

# file: pull_request_template.yml
<!-- 
Please refer to the commitizen documentation if you are unfamiliar with commitizen. Commitizen formalizes git commit messages for the sake of automation and version management. 

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

Additionally, please build and test your changes before submitting a PR. Thanks :)
-->
<!-- STEPS -->
<!-- 0) Make your PR title the conventional commit type/scope/description
<!-- 1) Optionally, insert a body paragraph here describing the nature of your commit. -->
This chunk of work introduces awesome bells and whistles that are super pleasing to the human ear.

<!-- 2a) Does this PR address a particular issue? List them here. -->
Fixes: #104, #105
<!-- 2b) Does this PR introduce breaking changes? Include the following flag, with an optional description -->
BREAKING CHANGES[: optional description]

@iiian , thanks for your response , but it still doesn't answer my question 😅 Does it enforce the conventional commit syntax on the commit title or the PR title?

iiian commented 2 months ago

Seems like it's only PR title. I thought you mentioned you had a ci-bot that would handle making a conventional commit when dropping it on main?

sansyrox commented 2 months ago

Seems like it's only PR title

PR title is the perfect behavior.

I thought you mentioned you had a ci-bot that would handle making a conventional commit when dropping it on main?

Nope, we don't have any as of now 😅