Summary: | webkitpy: Add a --no-comment switch to webkit-patch land | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Carlos Alberto Lopez Perez <clopez> | ||||||
Component: | Tools / Tests | Assignee: | Carlos Alberto Lopez Perez <clopez> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | aakash_jain, ap, dewei_zhu, ews-watchlist, glenn, jbedard, rniwa, ross.kirsling, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Carlos Alberto Lopez Perez
2020-10-21 19:04:33 PDT
Created attachment 412060 [details]
Patch
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. 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. (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. (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. (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. 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. 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. Created attachment 412967 [details]
Patch
v2: dont close the bug without commenting
Comment on attachment 412967 [details] Patch Clearing flags on attachment: 412967 Committed r269513: <https://trac.webkit.org/changeset/269513> All reviewed patches have been landed. Closing bug. |