Bug 57579 – webkit-patch is too strict about bug URL formatting

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Adam Roben (:aroben)]

webkit-patch is too strict about bug URL formatting

[Adam Roben (:aroben)]

Created attachment 87786 [details]
Patch

[Adam Barth]

Comment on attachment 87786 [details]
Patch

We changed this recently because other folks were sad that we were being too loose.  We should be sure we're making progress here instead of just flip/flopping between what two different camps want.

[Adam Barth]

https://bugs.webkit.org/show_bug.cgi?id=56989 is the earlier bug.

[Adam Roben (:aroben)]

See my comments in bug 56989 for why I think this is justified.

[Adam Barth]

I don't have strong feelings about this topic.

[Darin Adler]

Comment on attachment 87786 [details]
Patch

Seems fine.

[Ojan Vafai]

I'm not totally opposed to this change, but I don't really see the benefit in supporting different changelog formats. IMO, it helps the project if we're all consistent in how we write changelogs.

Some pros of consistency:
-easier to scan changelog entries quickly
-easier to write tooling for dealing with changelog descriptions
-easier for people new to the project to understand them and understand how to write their own descriptions

[Adam Roben (:aroben)]

I agree consistency is a good thing. I feel a little guilty every time I write a non-standard ChangeLog. I was just surprised that the tools started encouraging the format so strongly. And, as stated in bug 56989 comment 15, it seems like a bug that webkit-patch can no longer figure out a bug number for r81850.

[Ojan Vafai]

(In reply to comment #8)
> And, as stated in bug 56989 comment 15, it seems like a bug that webkit-patch can no longer figure out a bug number for r81850.

I guess this is the bit I don't agree with. The bug is only used to update/close the bug. But the patch wasn't actually fixing the bug. The bug is relevant to understanding the patch, but webkit-patch shouldn't be updating the bug.

[Adam Roben (:aroben)]

(In reply to comment #9)
> (In reply to comment #8)
> > And, as stated in bug 56989 comment 15, it seems like a bug that webkit-patch can no longer figure out a bug number for r81850.
> 
> I guess this is the bit I don't agree with. The bug is only used to update/close the bug. But the patch wasn't actually fixing the bug. The bug is relevant to understanding the patch, but webkit-patch shouldn't be updating the bug.

In that case --no-close seems appropriate. It would still be helpful to have webkit-patch add a comment to the bug even if you aren't closing it.

[Adam Roben (:aroben)]

Given that Darin and Eric seem to agree this patch is an improvement (see Eric's comment in bug 56989 comment 16) and that other people are running into trouble because of this (c.f. bug 57605), I'm going to land this patch. We can keep discussing further, but it seems good to make stylebot etc. stop complaining about ChangeLog formats that have long been in use (and worked just fine until this week).

[Adam Roben (:aroben)]

Committed r82681: <http://trac.webkit.org/changeset/82681>

[Ojan Vafai]

(In reply to comment #11)
> Given that Darin and Eric seem to agree this patch is an improvement (see Eric's comment in bug 56989 comment 16) and that other people are running into trouble because of this (c.f. bug 57605), I'm going to land this patch. We can keep discussing further, but it seems good to make stylebot etc. stop complaining about ChangeLog formats that have long been in use (and worked just fine until this week).

Sorry, I though the patch had already been committed. I was just trying to understand what makes sense going forward.

[Adam Roben (:aroben)]

Don't worry, I didn't think you were trying to hold things up. :-) And I'm glad you want to improve the tools, too.