[webkit-reviews] review granted: [Bug 171085] commit-log-editor should respect the git editor if one is set : [Attachment 307665] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Apr 20 17:23:42 PDT 2017
Daniel Bates <dbates at webkit.org> has granted Conrad Shultz
<conrad_shultz at apple.com>'s request for review:
Bug 171085: commit-log-editor should respect the git editor if one is set
https://bugs.webkit.org/show_bug.cgi?id=171085
Attachment 307665: Patch
https://bugs.webkit.org/attachment.cgi?id=307665&action=review
--- Comment #8 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 307665
--> https://bugs.webkit.org/attachment.cgi?id=307665
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=307665&action=review
> Tools/ChangeLog:10
> + If git is available, consider GIT_LOG_EDITOR and any global git
editor preference when deciding which editor to present.
> + We examine the global editor preference since that may be set
automatically by installers or third-party tools.
Nit: git => Git
(You mention Git twice in line 9)
It is an unwritten convention that we tend to wrap ChangeLog lines that exceed
~100 characters.
> Tools/Scripts/commit-log-editor:97
> +if (isGit()) {
> + $editor = $ENV{GIT_LOG_EDITOR};
> + $editor = `git config --global --get core.editor` if !$editor;
> +}
The behavior of this code to query the global git core editor may surprise a
person that never defines GIT_LOG_EDITOR, SVN_LOG_EDITOR, or CVS_LOG_EDITOR
and uses commit-log-editor located in a Git checkout of WebKit in projects that
use other version control systems because isGit() will return true (since the
WebKit checkout is Git-based). Though a similar surprise may arise if a person
just set GIT_LOG_EDITOR when using commit-log-editor in such a scenario.
More information about the webkit-reviews
mailing list