[dev-servo] Consider using GitCop
Friendly ping to mailing list about https://github.com/servo/servo/issues/16138, which I remembered because of https://github.com/servo/servo/pull/17138#issuecomment-305733987 In the following screenshot, you can see one doesn't even know what that commit is supposed to do from its title, because it is way too long to be informative. https://irccloud.mozilla.com/file/FJZOS7ii/Capture%20d’écran%202017-06-02%20à%2011.16.43.png I also want to argue that the very "bug X - " prefix is counterproductive in Servo commits, given that's not linkified anywhere, and thus not descriptive either. ___ dev-servo mailing list dev-servo@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-servo
Re: [dev-servo] Consider using GitCop
On 6/2/17 5:18 AM, Anthony Ramine wrote: In the following screenshot, you can see one doesn't even know what that commit is supposed to do from its title, because it is way too long to be informative. OK, what is the proposed cap on first line of commit message? Everything I've seen suggests the first line should be a one-sentence simple summary of what's going on. The fact that github insists on cutting it off to pretty short lengths, shorter than most sentences, is a bit unfortunate. Or concretely, what would you say one should use instead of "Being affected by id selectors should not prevent an element from being inserted in the style sharing cache" as a first line? I also want to argue that the very "bug X - " prefix is counterproductive in Servo commits If we don't propagate those servo commits as separate commits to the Gecko side, then I agree. I've just been putting the full bugzilla link in the non-first line of the commit message. -Boris ___ dev-servo mailing list dev-servo@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-servo
Re: [dev-servo] Consider using GitCop
Note that in most cases "bug X" isn't deliberate, it's due to `git am`ing a patch extracted from a gecko repo and forgetting to remove the Gecko stuff. That said, it does get linkified on the gecko side if it's in the PR title. I'm not very fond of the length limit imposed by GitCop. -Manish Goregaokar On Fri, Jun 2, 2017 at 8:00 AM, Boris Zbarsky wrote: > On 6/2/17 5:18 AM, Anthony Ramine wrote: > >> In the following screenshot, you can see one doesn't even know what that >> commit is supposed to do from its title, because it is way too long to be >> informative. >> > > OK, what is the proposed cap on first line of commit message? > > Everything I've seen suggests the first line should be a one-sentence > simple summary of what's going on. The fact that github insists on cutting > it off to pretty short lengths, shorter than most sentences, is a bit > unfortunate. > > Or concretely, what would you say one should use instead of "Being > affected by id selectors should not prevent an element from being inserted > in the style sharing cache" as a first line? > > I also want to argue that the very "bug X - " prefix is counterproductive >> in Servo commits >> > > If we don't propagate those servo commits as separate commits to the Gecko > side, then I agree. I've just been putting the full bugzilla link in the > non-first line of the commit message. > > -Boris > > ___ > dev-servo mailing list > dev-servo@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-servo > ___ dev-servo mailing list dev-servo@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-servo
Re: [dev-servo] Consider using GitCop
> Le 2 juin 2017 à 17:00, Boris Zbarsky a écrit : > > On 6/2/17 5:18 AM, Anthony Ramine wrote: >> In the following screenshot, you can see one doesn't even know what that >> commit is supposed to do from its title, because it is way too long to be >> informative. > > OK, what is the proposed cap on first line of commit message? > > Everything I've seen suggests the first line should be a one-sentence simple > summary of what's going on. The fact that github insists on cutting it off > to pretty short lengths, shorter than most sentences, is a bit unfortunate. Everything I've seen suggest the first line to be a *short* sentence, Git itself recommends it, and Github truncates because of this reason AFAIK. https://git-scm.com/docs/git-commit > Though not required, it’s a good idea to begin the commit message with a > single short (less than 50 character) line summarizing the change, followed > by a blank line and then a more thorough description. The text up to the > first blank line in a commit message is treated as the commit title, and that > title is used throughout Git. For example, git-format-patch[1] turns a commit > into email, and it uses the title on the Subject line and the rest of the > commit in the body. http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html https://github.com/erlang/otp/wiki/Writing-good-commit-messages (wink wink) https://github.com/git/git/blob/master/Documentation/SubmittingPatches#L95-L99 > Or concretely, what would you say one should use instead of "Being affected > by id selectors should not prevent an element from being inserted in the > style sharing cache" as a first line? I see two separate issues with that title: it doesn't describe the change, but what was wrong before it, and it is indeed rather long. Not knowing what the actual patch is about, here are a couple suggestions: - Fix style sharing of elements affected by id selectors - Share styles of elements affected by id selectors (only?) >> I also want to argue that the very "bug X - " prefix is counterproductive in >> Servo commits > > If we don't propagate those servo commits as separate commits to the Gecko > side, then I agree. I've just been putting the full bugzilla link in the > non-first line of the commit message. Links are good, thanks. I don't particularly mind the actual bug numbers, I just think they would be easier to ignore (and would waste less screen estate) if they were added as suffixes of the commit title. ___ dev-servo mailing list dev-servo@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-servo
Re: [dev-servo] Consider using GitCop
> I'm not very fond of the length limit imposed by GitCop. The length is configurable. On Fri, Jun 2, 2017 at 8:50 AM Anthony Ramine wrote: > > > > Le 2 juin 2017 à 17:00, Boris Zbarsky a écrit : > > > > On 6/2/17 5:18 AM, Anthony Ramine wrote: > >> In the following screenshot, you can see one doesn't even know what > that commit is supposed to do from its title, because it is way too long to > be informative. > > > > OK, what is the proposed cap on first line of commit message? > > > > Everything I've seen suggests the first line should be a one-sentence > simple summary of what's going on. The fact that github insists on cutting > it off to pretty short lengths, shorter than most sentences, is a bit > unfortunate. > > Everything I've seen suggest the first line to be a *short* sentence, Git > itself recommends it, and Github truncates because of this reason AFAIK. > > https://git-scm.com/docs/git-commit > > > Though not required, it’s a good idea to begin the commit message with a > single short (less than 50 character) line summarizing the change, followed > by a blank line and then a more thorough description. The text up to the > first blank line in a commit message is treated as the commit title, and > that title is used throughout Git. For example, git-format-patch[1] turns a > commit into email, and it uses the title on the Subject line and the rest > of the commit in the body. > > http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html > https://github.com/erlang/otp/wiki/Writing-good-commit-messages (wink > wink) > > https://github.com/git/git/blob/master/Documentation/SubmittingPatches#L95-L99 > > > Or concretely, what would you say one should use instead of "Being > affected by id selectors should not prevent an element from being inserted > in the style sharing cache" as a first line? > > I see two separate issues with that title: it doesn't describe the change, > but what was wrong before it, and it is indeed rather long. Not knowing > what the actual patch is about, here are a couple suggestions: > > - Fix style sharing of elements affected by id selectors > - Share styles of elements affected by id selectors (only?) > > >> I also want to argue that the very "bug X - " prefix is > counterproductive in Servo commits > > > > If we don't propagate those servo commits as separate commits to the > Gecko side, then I agree. I've just been putting the full bugzilla link in > the non-first line of the commit message. > > Links are good, thanks. I don't particularly mind the actual bug numbers, > I just think they would be easier to ignore (and would waste less screen > estate) if they were added as suffixes of the commit title. > > ___ > dev-servo mailing list > dev-servo@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-servo > ___ dev-servo mailing list dev-servo@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-servo
Re: [dev-servo] Consider using GitCop
On 6/2/17 11:50 AM, Anthony Ramine wrote: Though not required, it’s a good idea to begin the commit message with a single short (less than 50 character) Just so we're clear, 50 characters is about 9 words. - Fix style sharing of elements affected by id selectors No, that's not what it does. Doing _that_ requires 4 separate changes, of which this is but one. - Share styles of elements affected by id selectors (only?) No, that's not what it does either. Again, it's a step on the way to doing that. It seems to me like the "50 character" rule is pretty hostile to fine-grained changesets, basically. -Boris ___ dev-servo mailing list dev-servo@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-servo
Re: [dev-servo] Consider using GitCop
> Le 2 juin 2017 à 18:22, Boris Zbarsky a écrit : > > On 6/2/17 11:50 AM, Anthony Ramine wrote: >>> Though not required, it’s a good idea to begin the commit message with a >>> single short (less than 50 character) > > Just so we're clear, 50 characters is about 9 words. I haven't said we should use 50 chars, I was quoting the docs, that's all. I want a line length limit, I didn't say I want it to be that short. I would like our limit to be the same as the threshold over which GH truncates, which is 80 I think. >> - Fix style sharing of elements affected by id selectors > > No, that's not what it does. Doing _that_ requires 4 separate changes, of > which this is but one. Well as I said, I don't know what the actual patch is about and the original title didn't help. >> - Share styles of elements affected by id selectors (only?) > > No, that's not what it does either. Again, it's a step on the way to doing > that. What was that step? The title you pasted doesn't say what it was about, it only said what was wrong without it. > It seems to me like the "50 character" rule is pretty hostile to fine-grained > changesets, basically. Agree about 50, not about the concept of a limit in general. I often do fine-grained changesets and it very rarely was an issue. In the few cases it was, I took it as a hint that I should be writing a longer explanation in the actual message that wouldn't fit the title anyway. ___ dev-servo mailing list dev-servo@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-servo
Re: [dev-servo] Consider using GitCop
On 6/2/17 12:27 PM, Anthony Ramine wrote: What was that step? The title you pasted doesn't say what it was about, it only said what was wrong without it. That was all. I just removed the if condition in question around the insertion. I've tried rewording as this: Allow inserting elements into the style sharing cache even when they are affected by id selectors. but that's still clearly longer than 50 chars. Agree about 50, not about the concept of a limit in general. OK, that's fair. I don't have a problem with a limit a priori. -Boris ___ dev-servo mailing list dev-servo@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-servo
Re: [dev-servo] Consider using GitCop
$ echo -n "Allow style sharing elements with ID selectors" | wc -c 46 On Fri, Jun 2, 2017 at 9:31 AM Boris Zbarsky wrote: > On 6/2/17 12:27 PM, Anthony Ramine wrote: > > What was that step? The title you pasted doesn't say what it was about, > it only said what was wrong without it. > > That was all. I just removed the if condition in question around the > insertion. > > I've tried rewording as this: > >Allow inserting elements into the style sharing cache even when they > are affected by id selectors. > > but that's still clearly longer than 50 chars. > > > Agree about 50, not about the concept of a limit in general. > > OK, that's fair. I don't have a problem with a limit a priori. > > -Boris > > ___ > dev-servo mailing list > dev-servo@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-servo > ___ dev-servo mailing list dev-servo@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-servo
Re: [dev-servo] Consider using GitCop
On Fri, Jun 2, 2017 at 8:00 AM, Boris Zbarsky wrote:
> On 6/2/17 5:18 AM, Anthony Ramine wrote:
>
>> In the following screenshot, you can see one doesn't even know what that
>> commit is supposed to do from its title, because it is way too long to be
>> informative.
>>
>
> OK, what is the proposed cap on first line of commit message?
>
IIRC the culture around length limits is mostly a cargo cult. If you squint
hard enough, you can trace the origins of these line limits to:
* The prevalence of email to exchange patches (like Linux, which is what
Git was invented for) plus the fact that some email encodings wrap long
lines
* IBM punch cards having 80 columns (this was later carried forward to TTYs
and has been cargo culted through subsequent decades)
In other words, the [short] line limits or not relevant to many modern
workflows. It is unfortunate tools like GitHub are participating in this
cargo cult.
As someone who spends a lot of time writing tools, I really despise when
tools dictate implementation details: tools should be subservient to their
users, not the other way around.
>
> Everything I've seen suggests the first line should be a one-sentence
> simple summary of what's going on. The fact that github insists on cutting
> it off to pretty short lengths, shorter than most sentences, is a bit
> unfortunate.
>
> Or concretely, what would you say one should use instead of "Being
> affected by id selectors should not prevent an element from being inserted
> in the style sharing cache" as a first line?
>
> I also want to argue that the very "bug X - " prefix is counterproductive
>> in Servo commits
>>
>
> If we don't propagate those servo commits as separate commits to the Gecko
> side, then I agree. I've just been putting the full bugzilla link in the
> non-first line of the commit message.
I agree you are in kind of a bind here because you probably aren't going to
convince GitHub to change their UI. However, I believe the full commit
message is in the DOM. At least it is for the example in the original post.
I'm sure someone could come up with a WebExtension or Greasemonkey script
to hack the UI to show what you want. I wouldn't be surprised if someone
already has!
Also, we don't have these "lack of control over tools" problems with
hg.mozilla.org. While the UI isn't as clean as GitHub's, we don't truncate
the commit message. And where it is appropriate, we also supplement views
with relevant metadata, such as links to bugs, reviewer name, link to
automation results, etc. See https://hg.mozilla.org/mozilla-central/rev/
39e5304d676d for an example.
You may find the https://hg.mozilla.org/projects/converted-servo-linear
repo useful: it is all the changesets comprising the servo/ directory in
mozilla-central before they are overlayed into mozilla-central. But it has
all the merges collapsed. If a straight conversion of the Git repo to
Mercurial (so you aren't crippled by GitHub's UI) would be useful, I've
created a one-off at
https://hg.mozilla.org/users/gszorc_mozilla.com/servo-git-conversion. If
you want it done in a more formal manner (ongoing conversion, links back to
GitHub, etc), it shouldn't be too much effort. File a Servo VCS Sync bug.
While I'm advocating for the benefits of the Mercurial web UI, that search
box in the top right is useful. It accepts many Mercurial revset functions (
https://hg.mozilla.org/mozilla-central/help/revisions). This means you have
a query language and can type things like |file("components/layout/**") &
reviewer(bholley)| to perform powerful searches. Again, we have full
control over that UI, can implement custom query functions, and can easily
submit patches to upstream Mercurial. So if something will make your life
easier, please ask for it by filing an hg.mozilla.org bug.
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo
Re: [dev-servo] Consider using GitCop
On 6/2/17 12:34 PM, Michael Howell wrote: $ echo -n "Allow style sharing elements with ID selectors" | wc -c Misleading, unfortunately, because that changeset on its own doesn't allow anything of the sort. -Boris ___ dev-servo mailing list dev-servo@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-servo
