webkitpy: Add a --no-comment switch to webkit-patch land

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Carlos Alberto Lopez Perez]

When using webkit-patch land to land a local patch, the tool always comment on the bug that is pointed on the changelog.

Sometimes I find desirable to not add a comment automatically, for example when landing a gardening patch or a build-fix patch where I use the original bug that caused the issue on the changelog.
I prefer to add manually a comment after landing the patch, but the tool doesn't me allow to skip adding this automatic comment.

[Carlos Alberto Lopez Perez]

Created attachment 412060 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 412060 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=412060&action=review

> Tools/Scripts/webkitpy/tool/steps/closebugforlanddiff.py:59
> +                    _log.info("Closing without commenting bug %s" % bug_id)
> +                    self._tool.bugs.close_bug_as_fixed(bug_id)

It's not great to close a bug without commenting.
We should probably not close the bug or leave a comment regardless of the option in this case.

[Aakash Jain]

Agree with Ryosuke. It doesn't seems like a good idea to close a bug without commenting. People can forget to add a manual comment later on, or miss including the specific commit link in the comment.

Also the use-case isn't very clear to me. Can you please give some specific examples where such a functionality of no-comment was required?
In case of a build-fix commit, what's the problem in adding the automated comment (which includes the link to the fix)? The comment will let people watching that bug know immediately that a fix has been landed.

[Carlos Alberto Lopez Perez]

(In reply to Aakash Jain from comment #3)
> Also the use-case isn't very clear to me. Can you please give some specific
> examples where such a functionality of no-comment was required?
> In case of a build-fix commit, what's the problem in adding the automated
> comment (which includes the link to the fix)? The comment will let people
> watching that bug know immediately that a fix has been landed.

Example: https://bugs.webkit.org/show_bug.cgi?id=218017#c11

I usually prefer to give some context along with the commit number on the bugs, so I end commenting twice because the tool forces to leave that automatic comment with just the commit number.

Note that this will be an optional switch, the one using it would do that on purpose so I expect she will be diligent enough to put her own comment on time.

[Radar WebKit Bug Importer]

<rdar://problem/70790813>

[Aakash Jain]

(In reply to Carlos Alberto Lopez Perez from comment #4)
> I usually prefer to give some context along with the commit number on the bugs, so I end commenting twice because the tool forces to leave that automatic comment with just the commit number.
You also have the option to use 'svn commit' directly if you don't want 'webkit-patch land' to interact with the bug. Is there any specific benefit of using 'webkit-patch land' over 'svn commit' if you don't want Bugzilla interaction.

[Jonathan Bedard]

(In reply to Aakash Jain from comment #6)
> (In reply to Carlos Alberto Lopez Perez from comment #4)
> > I usually prefer to give some context along with the commit number on the bugs, so I end commenting twice because the tool forces to leave that automatic comment with just the commit number.
> You also have the option to use 'svn commit' directly if you don't want
> 'webkit-patch land' to interact with the bug. Is there any specific benefit
> of using 'webkit-patch land' over 'svn commit' if you don't want Bugzilla
> interaction.

I'm not sure we should encourage folks to use svn commit directly, that bypasses adding the commit message. Not a huge deal for SVN, but a pretty big problem for the Git future we're working towards.

[Jonathan Bedard]

I think Carlos has a good use case here. I want to give Ryosuke and Aakash a chance to rebuttal, but "I want to comment on Bugzilla myself" seems like a valid use case....and I would much rather an engineer use webkit-patch in that case instead of committing directly.

[Ryosuke Niwa]

Comment on attachment 412060 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=412060&action=review

>> Tools/Scripts/webkitpy/tool/steps/closebugforlanddiff.py:59
>> +                    self._tool.bugs.close_bug_as_fixed(bug_id)
> 
> It's not great to close a bug without commenting.
> We should probably not close the bug or leave a comment regardless of the option in this case.

r- because of this point.

[Carlos Alberto Lopez Perez]

Created attachment 412967 [details]
Patch

v2: dont close the bug without commenting

[Carlos Alberto Lopez Perez]

Comment on attachment 412967 [details]
Patch

Clearing flags on attachment: 412967

Committed r269513: <https://trac.webkit.org/changeset/269513>

[Carlos Alberto Lopez Perez]

All reviewed patches have been landed.  Closing bug.