Re: [webkit-dev] Update commit log template to add placeholder for explanation of why a patch fixes a bug

2022-11-17 Thread David Kilzer via webkit-dev
On Nov 17, 2022, at 3:17 PM, Michael Catanzaro  wrote:

> On Thu, Nov 17 2022 at 02:48:04 PM -0800, Ryosuke Niwa  
> wrote:
>> But every change in WebKit comes with a Bugzilla bug.
> 
> Certainly most do, but some counterexamples:
> 
> * Unreviewed build fixes sometimes do not reference a bug
> * When fixing a new compiler warning or build failure, I often reference the 
> bug that introduced the problem, rather than reporting a new one just to have 
> a new hyperlink for the commit message
> * We probably don't need a bug report for stuff like "update my email address 
> in contributors.json" or "fix typo in comment”

Yes, this is why I stated this in the original message:

>>> Note that the line may be simply be deleted when it doesn’t apply (such as 
>>> gardening, a build fix, etc.).


I don’t think it’s too burdensome to delete a couple lines of text for the 
above kinds of fixes if the result is better commit messages overall.

Dave

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


[webkit-dev] Update commit log template to add placeholder for explanation of why a patch fixes a bug

2022-11-17 Thread David Kilzer via webkit-dev
Hi,

The following PR adds placeholder text in the commit log template to remind 
authors to explain why a change fixes a bug:

Bug 248012: Update commit message template to request a brief explanation of 
why a PR fixes the bug
>
>

It looks like this:

Need a short description (OOPS!).
Need the bug URL (OOPS!).
Include a Radar link (OOPS!).

Reviewed by NOBODY (OOPS!).

Short explanation why this fixes the bug (OOPS!).

* path/to/File.cpp:
(Method::Name):

Since this is effectively a policy change, Jonathan suggested I post it here 
first before the change is landed.

I believe many WebKit contributors already do this anyway, and it’s always been 
a best-practice to explain why a change fixes a bug (vs. what a patch does, 
which is usually obvious by reading the code), so this text just serves as a 
reminder to write a short explanation for every relevant patch.

Note that the line may be simply be deleted when it doesn’t apply (such as 
gardening, a build fix, etc.).

Any feedback on this change?

Dave

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev