search

conradludgate / interim

English date parser. Fork of chrono-english
MIT License
5 stars 2 forks source link

Better commit messages #11

Closed martinvonz closed 2 weeks ago

martinvonz commented 2 weeks ago

Thanks for providing a maintained alternative to chrono-english. I found this crate from https://rustsec.org/advisories/RUSTSEC-2024-0395. To make people more confident in the project, I think it needs better commit messages. Commit messages saying no more than "stuff" (649ed32) or "time" (3ae5756) should not be allowed to make it into the main branch IMO. "fix timezone support" (de3973d) is slightly better but it really should explain what was broken. The reason I noticed was that I was trying to find a commit that changed something about time zones (to figure out why my tests started failing after switching from chrono-english). See https://cbea.ms/git-commit/ for more detailed suggestions.

Thanks for considering. I hope the above doesn't sound too harsh.

erin-desu commented 2 weeks ago

I think quite the opposite actually. Such commit messages make you look into the changes and actually evaluate the code yourself.

conradludgate commented 2 weeks ago

To make people more confident in the project, I think it needs better commit messages.

This project was effectively born as a side project to rewrite and improve the code. I did rush through the commits and it wasn't originally intended to go beyond something that only I was using.

The reason I noticed was that I was trying to find a commit that changed something about time zones (to figure out why my tests started failing after switching from chrono-english).

This project is effectively a complete rewrite. I would wager that the commits would not be of value here. Could you open a new issue for this discrepancy?

martinvonz commented 2 weeks ago

I think quite the opposite actually. Such commit messages make you look into the changes and actually evaluate the code yourself.

I suspect not many people feel that way. And it's much easier for you to just ignore detailed commit messages (than it is for others to infer them when they're missing). You can even create a git alias for git log --pretty=<something> that just shows the first line (or nothing at all) if you prefer.

Also, even if it's possible to look at the patch and figure out what it does, you lose the record of why it was done.

This project was effectively born as a side project to rewrite and improve the code. I did rush through the commits and it wasn't originally intended to go beyond something that only I was using.

That's fair. (Well, kind of :) I would still recommend including a bit more context, but I work on source control stuff, so...)

This project is effectively a complete rewrite. I would wager that the commits would not be of value here. Could you open a new issue for this discrepancy?

I filed #12 for that. Thanks.

ilyagr commented 2 weeks ago

Thanks @conradludgate for making interim!

@erin-desu , there are plenty of reasons why a project might have unhelpful commit descriptions, but there's something particularly dispiriting about trying to claim that it's a good thing.

True, it's possible to write commit descriptions that would be worse than no description, but that would be a different problem. If you feel like you don't know how to write a better commit description than no description, that's a fair reason to not write them, and you are under no obligation to change that (unless you work with people for whom it's a problem and who might obligate you). Still, claiming that it is impossible feels like a step too far.

This is in addition to Martin's point that there definitely exist people who appreciate nice commit messages. In my opinion, this is clearer; even if you argue with my other point, it is harder to argue that I do not exist or that I do not appreciate nice commit messages, unless you fully descend into solipsism and its standard of what is and isn't clear.