rust-windowing / winit

Window handling library in pure Rust
https://docs.rs/winit/
Apache License 2.0
4.68k stars 881 forks source link

Add commit message lints #3545

Open kchibisov opened 6 months ago

kchibisov commented 6 months ago

Unfortunatelly, it's impossible for everyone to at least wrap their commit messages. After numerous warnings folks can't still do such things.

Thus, the squash merge will be disabled, and the only option will be rebase with the commit lint enabled for each commit in the series.

madsmtm commented 6 months ago

I'm probably one of the offenders here, but disagree that this is a priority, IMO just get your editor / terminal / git to wrap the lines for you if you want them wrapped.

That said, I'm fine with CI jobs or something to fix this, but I believe there is great value in not telling contributors to rebase, and squashing instead.

kchibisov commented 6 months ago

That said, I'm fine with CI jobs or something to fix this, but I believe there is great value in not telling contributors to rebase, and squashing instead.

You can do that yourself if you want to, I can't be bothered to deal with these commit messages anymore.

I told about that numerous times during the year(I think), so no it's a priority, since mostly all commit messages expept for the ones from @notgull and me are formatted like garbage.

daxpedda commented 6 months ago

I agree that this is not a priority. It would also be good to agree on something and write it down somewhere. Personally I couldn't care less about wrapping commit messages (to a degree ofc), but if we agree on something and write it down somewhere I'm happy to oblige.

I don't like squashing either, fast-forward-merging anybody?

kchibisov commented 6 months ago

I mean there's nothing to agree, it's a standard to wrap body at 72 and title around 50 chars. I also asked to strip garbage github adds, because I can't be bothered to read any of that and it just leaves a feeling that no one cares to write anything useful in the end of the day.

image

Or like that

image

madsmtm commented 6 months ago

All I see is an issue with your terminal window being too wide / git not automatically wrapping the message to a readable width like any normal text-box in a modern system otherwise does ;).

Or do you have other complaints with the commit messages?

kchibisov commented 6 months ago

All I see is an issue with your terminal window being too wide / git not automatically wrapping the message to a readable width like any normal text-box in a modern system otherwise does ;).

Ofc, just read yours and then e.g. John's or mine. Or read the alacritty's messages. It's feels like you throw garbage which no-one cares about and then dance around the tree because your thing got merged.

And no, it's not an issue with my system in particular, you should just follow what git uses by default and at least put a bit of effort into writing about the work you've done.

daxpedda commented 6 months ago

It's feels like you throw garbage which no-one cares about and then dance around the tree because your thing got merged.

I don't believe this language is appropriate. Please remain on topic.


what git uses by default

I'm not aware of git imposing any rules by default? What are you referring to here?

kchibisov commented 6 months ago

I don't believe this language is appropriate. Please remain on topic.

I was polite about this during the year. Mostly all my suggestions were ignored and they don't want to improve on purpose resulting in lower code quality in the project, because git commit messages are part of that. Given that I can't stand it anymore I take it personally and treat as a threat it as power abuse since 99% of the time I rewrite their commit messages for them, which they still not appreciate and continue to merge arbitrary garbage. If they can't behave, I'll make the rules stricter for everyone, including good participants.

I'm not aware of git imposing any rules by default? What are you referring to here?

50 and 72 wrapping is de-facto wrapping in git. I don't force 50, you can still write a bit more in title, by like 10 chars, but general aim is 50.

kchibisov commented 6 months ago

I've disabled squash and forced explicit review on each PR.

madsmtm commented 5 months ago

I've set up a Git commit-msg hook now.

kchibisov commented 5 months ago

I've set up a Git commit-msg hook now.

For yourself or what do you mean by that?

madsmtm commented 5 months ago

Yeah, for myself

madsmtm commented 5 months ago

I'll try to set up a CI job that checks this, so that we can re-enable squash merging

kchibisov commented 5 months ago

The issue is that CI won't be able to prevent squash merging, because you write commit messages on github, thus CI won't run for it.

The only way CI for commits can work is for rebase itself. The squash will be re-enabled eventually.

daxpedda commented 5 months ago

I think we should agree on some rules in https://github.com/rust-windowing/winit/pull/3549 first.

kchibisov commented 5 months ago

I think the point is to lint on width at least. The lints for commit messages are present in e.g. systemd, etc, iirc.