crate-ci / committed

Nitpicking commit history since beabf39
Apache License 2.0
107 stars 7 forks source link

RFE: option to require non-capitalized subjects #408

Open scop opened 3 weeks ago

scop commented 3 weeks ago

Requiring commit subjects not to be capitalized is a common practice especially with conventional commit implementations. There does not seem to be a way to enforce this with committed, and it would be nice to have.

Setting subject_capitalized to false does not appear to result in capitalized subjects flagged as an error, it just does not require them to be capitalized then. Perhaps it could be changed to a tri-state option so that if not defined, no particular case is required; if true, capitalization required; if false, non-capitalization required.

Some references

epage commented 3 weeks ago

I'm fine adding a check for non-capitalized subjects. I'd prefer to not make a breaking change in the configuration format though. I'd be open to other ideas.

scop commented 2 weeks ago

Thanks for considering!

Adding subject_uncapitalized would be one option, but clunky: does not sound good, and weirdness ensues if both subject_capitalized and subject_uncapitalized are present.

Adding subject_case config with options capitalized and uncapitalized could be better: this, if present, would override subject_capitalized. subject_capitalized could be deprecated in favor of this.

epage commented 2 weeks ago

Due to some recent work, Rust lints are heavily on my mind since I last spoke. I also forgot that I've been toying with moving in a lint level-like direction.

Adding subject_uncapitalized would be one option, but clunky: does not sound good, and weirdness ensues if both subject_capitalized and subject_uncapitalized are present.

This is the approach rustc takes for lint naming

The basic rule is: the lint name should make sense when read as "allow lint-name" or "allow lint-name items". For example, "allow deprecated items" and "allow dead_code" makes sense, while "allow unsafe_block" is ungrammatical (should be plural).

clippy even has mutually exclusive lints, though they limit those to a specific group, restrictions.

Hmm, however, the problem is with defaults. With clippy, restrictions are never enabled by default so if you enable one, you don't have to disable the other. That doesn't work here.

Sounds like we do need to go with an enumeration.

Thinking more on this, I feel like maybe it is more correct to say sentence_case (sigh why did I use snake_case) or sentence for what we currently have but that or its opposite make it sound like we'll check the case of the following words.

epage commented 2 weeks ago

: this, if present, would override subject_capitalized. subject_capitalized could be deprecated in favor of this.

When I've done these migrations in the past, I've errored out if both are explicitly set because that is ambiguous. I don't think we should prefer one over the other.