[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]

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.