Replies: 12 comments
-
cc @aneeshusa |
Beta Was this translation helpful? Give feedback.
-
This is probably worth taking to the mailing list. I have not heard any opinions about commit titles from anybody else on the team; I personally have no issues with longer ones. |
Beta Was this translation helpful? Give feedback.
-
Line length is one thing, but gitcop was annoyingly rigid beyond that when I’ve had to deal with it in other repos. For example, each commit title much follow a pattern that includes a "type" or "category" to be picked in some arbitrary closed set. (What does |
Beta Was this translation helpful? Give feedback.
-
I'm pretty sure that particular part you mention (I hate it too) can be disabled. |
Beta Was this translation helpful? Give feedback.
-
@notriddle suggested this in #16009 I personally hate long commit titles (among other things). It should only give a gist. Anything beyond that can go into the summary. GitCop is probably worth trying it out? |
Beta Was this translation helpful? Give feedback.
-
On the contrary, I dislike having to shorten my commit messages to fix in GitHub's preferred limit, often at the expense of useful information. Sure, it could go in the body of the commit message, but if it's fine for critical information about the commit to be relegated to the body of the commit message, why is it important to have a good commit message first line at all? |
Beta Was this translation helpful? Give feedback.
-
"GitHub's preferred limit" is Git's own preferred limit in general. I very rarely see "critical information" in these overlong commit titles, and I believe putting more thought in the commit messages help in the long run. There is also a tooling issue, in which all Git tools expect short titles and then more explanations in the message. All projects I've contributed to that were using Git more or less followed the same recommendations, so I also consider doing the same thing is being a good FOSS citizen. |
Beta Was this translation helpful? Give feedback.
-
GitCop looks like a pretty way to address this and part of a complete solution if we decide it's a problem we need to solve. However, for the sake of people who might be doing Servo development primarily off-line or prefer to test their changes locally before sharing them with the world, I would strongly suggest adding the commit summary length test to the repo's built-in style lints (Tidy is one place we could, thoguht here are other viable options) if we deploy GitCop. That way, our existing CI processes would seamlessly include the test, and someone running the test suite locally would be able to more accurately predict whether their PR will have issues. At that point, GitCop is more of a pretty user interface and no additional changes will be required if the GitCop service goes away. As for whether this lint is actually a good idea... I'll let the people who actually have to stare at the commit history on a daily basis sort that out among themselves |
Beta Was this translation helpful? Give feedback.
-
I 100% agree with this, and if I don't get my line length limit on the sole basis that we actually need to implement it ourselves, I won't be mad. |
Beta Was this translation helpful? Give feedback.
-
@nox That is probably the most un-bikesheddy thing I have ever heard in a discussion like this 🚲 To clarify, this would be pretty straightforward to stick into Tidy as a mentored bug or an hour of reverting and tweaking and adding a test for the old commit message linting that was recently removed. I don't think the need to stick it in tidy should sway the decision process :) |
Beta Was this translation helpful? Give feedback.
-
Also we need to make sure this doesn't interfere with automatic sheriff backouts. |
Beta Was this translation helpful? Give feedback.
-
You all know I hate overly long commit titles. Maybe we should use https://gitcop.com.
Beta Was this translation helpful? Give feedback.
All reactions