How I review patches

Here is a write-up on how I review patches posted on the list, in the hope that knowing what to expect may help contributors [*0*]. But before going into exactly how I do my reviews, here is a short list of the goals of doing reviews. I review a patch (or a set of patches) to ensure that

Re: [PATCH] commit: Optimize number of lstat() calls

Johannes Schindelin writes: >> Besides, leaving the main index not refreshed would mean the user >> has to pay the refreshing cost when s/he runs other commands "git >> diff", "git status", etc. after "git commit" for the first time; >> so... > > I am not sure... when

Re: [PATCH] commit: Optimize number of lstat() calls

Hi Junio, On Fri, 3 Feb 2017, Junio C Hamano wrote: > Aside from whitespace breakage, I think we should never skip the > refreshing of the real index that is left after "git commit" > finishes. > > Between these two calls to refresh_cache(), the one that writes out > a temporary index that

Re: [PATCH] commit: Optimize number of lstat() calls

Aside from whitespace breakage, I think we should never skip the refreshing of the real index that is left after "git commit" finishes. Between these two calls to refresh_cache(), the one that writes out a temporary index that contains the contents of HEAD plus the contents of the working tree

[PATCH] Add --gui option to mergetool

* fix the discrepancy between difftool and mergetool where the former has the --gui flag and the latter does not by adding the functionality to mergetool * make difftool read 'merge.guitool' as a fallback, in accordance to the manpage for difftool: "git difftool falls back to git mergetool

Re: [PATCH] Remove --no-gui option from difftool usage string

On Fri, Feb 03, 2017 at 09:58:09PM -0800, Jacob Keller wrote: > On Fri, Feb 3, 2017 at 6:56 PM, Denton Liu wrote: > > The --no-gui option not documented in the manpage, nor is it actually > > used in the source code. This change removes it from the usage help > > that's

Re: [PATCH 00/12] completion: speed up refs completion

Jacob Keller writes: > On Thu, Feb 2, 2017 at 8:15 PM, Jacob Keller wrote: >> On Thu, Feb 2, 2017 at 6:53 PM, SZEDER Gábor wrote: >>> This series speeds up refs completion for large number of refs, partly >>> by giving up

Re: [PATCH] Remove --no-gui option from difftool usage string

On Fri, Feb 3, 2017 at 6:56 PM, Denton Liu wrote: > The --no-gui option not documented in the manpage, nor is it actually > used in the source code. This change removes it from the usage help > that's printed. > > Signed-off-by: Denton Liu > --- >

[L10N] Kickoff of translation for Git 2.12.0 round 1

Hi, Git v2.12.0-rc0 has been released, and it's time to start new round of git l10n. This time there are 200+ updated messages need to be translated since last update: l10n: git.pot: v2.12.0 round 1 (239 new, 15 removed) Generate po/git.pot from v2.12.0-rc0 for git v2.12.0 l10n round 1.

Re: [PATCH] git-parse-remote.sh: Remove op_prep argument

Pranit Bauva writes: > So if you want a better commit message then you could probably use this, > "parse-remote: remove reference to unused op_prep > > This argument was introduced in the commit 15a147e618 to help in > writing out the error message but then in commit

Re: [PATCH] tag: add a config option for setting --annotate by default

David Aguilar writes: > Make it easier for users to remember to annotate their tags. > Allow setting the default value for "--annotate" via the "tag.annotate" > configuration variable. > > Signed-off-by: David Aguilar > --- I do not care too strongly about

Re: [PATCH 00/12] completion: speed up refs completion

On Thu, Feb 2, 2017 at 8:15 PM, Jacob Keller wrote: > On Thu, Feb 2, 2017 at 6:53 PM, SZEDER Gábor wrote: >> This series speeds up refs completion for large number of refs, partly >> by giving up disambiguating ambiguous refs (patch 6) and partly by

[PATCH] Remove --no-gui option from difftool usage string

The --no-gui option not documented in the manpage, nor is it actually used in the source code. This change removes it from the usage help that's printed. Signed-off-by: Denton Liu --- git-difftool.perl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git

[PATCH] tag: add a config option for setting --annotate by default

Make it easier for users to remember to annotate their tags. Allow setting the default value for "--annotate" via the "tag.annotate" configuration variable. Signed-off-by: David Aguilar --- Documentation/config.txt | 5 + builtin/tag.c| 15 ---

Re: BUG: "git checkout -q -b foo origin/foo" is not quiet

On 02/03/2017 10:36 PM, Kevin Layer wrote: > Note that git version 1.8.3.1 is quiet and does not print the > "tracking remote" message. So what you are saying is, that git v1.8.3.1 *is* quiet, but v1.7.1 is not? Sounds like a fixed bug to me. I also checked with the latest version and it did not

Re: [PATCH] document behavior of empty color name

On Fri, Feb 03, 2017 at 10:12:13AM -0800, Junio C Hamano wrote: > > Right, I think that's the correct behavior. The empty color name is a > > real color ("none"), and you can put it in your list just like any other > > color. > > Makes me wonder if we have a non-empty string that spells the same

Re: [PATCH] git-parse-remote.sh: Remove op_prep argument

Hey Siddharth, On Fri, Feb 3, 2017 at 11:58 PM, Siddharth Kannan wrote: > - Remove the third argument of error_on_missing_default_upstream that is no > longer required > - FIXME to remove this argument was added in commit 045fac5845 This is not exactly correct.

Re: [PATCH] commit: Optimize number of lstat() calls

"Gumbel, Matthew K" writes: > When making a --only commit, original behavior was to do a full cache > update for the purposes of giving the pre-commit hook an up-to-date set > of stat data. That would result in long runtime for git-commit in a big > repo on NFS

[PATCH] commit: Optimize number of lstat() calls

When making a --only commit, original behavior was to do a full cache update for the purposes of giving the pre-commit hook an up-to-date set of stat data. That would result in long runtime for git-commit in a big repo on NFS (>60s for a 54k-file repo). With this change, when doing a --only

Re: git-scm.com status report

On Fri, Feb 03, 2017 at 09:23:33PM +, pedro rijo wrote: > Seems a good idea. I will start by going through some old prs/issues to > look for trash. If I do find some like the one I referred I will let you > know by mentioning you. After that I will have a look at simpler issues/prs. > > Let

[ANNOUNCE] Git v2.12.0-rc0

An early preview release Git v2.12.0-rc0 is now available for testing at the usual places. It is comprised of 441 non-merge commits since v2.11.0, contributed by 63 people, 19 of which are new faces. The tarballs are found at: https://www.kernel.org/pub/software/scm/git/testing/ The

Re: BUG: "git checkout -q -b foo origin/foo" is not quiet

Note that git version 1.8.3.1 is quiet and does not print the "tracking remote" message. On Fri, Feb 3, 2017 at 10:20 AM, Kevin Layer wrote: > $ git checkout -q -b foo origin/foo > Branch foo set up to track remote branch foo from origin. > > I said -q because I didn't want

Re: git-scm.com status report

On Fri, Feb 3, 2017 at 5:58 AM, Jeff King wrote: > On Thu, Feb 02, 2017 at 12:54:53AM -0600, Samuel Lijin wrote: > >> In theory, you could also dump the build artifacts to a GH Pages repo >> and host it from there, although I don't know if you would run up >> against any of the

Re: [PATCH v2] reset: add an example of how to split a commit into two

Jacob Keller writes: > +Split a commit into two:: Let's say "...into two (or more)" to match what appears in "SPLITTING COMMITS" section of "rebase -i" documentation. Yours is written as a sequence of more concrete steps than the existing one over there, so it may

[PATCH v2] reset: add an example of how to split a commit into two

From: Jacob Keller It is often useful to break a commit into multiple parts that are more logical separations. This can be tricky to learn how to do without the brute-force method if re-writing code or commit messages from scratch. Add a section to the git-reset

Re: [PATCH] reset: add an example of how to split a commit into two

On Fri, Feb 3, 2017 at 10:59 AM, Junio C Hamano wrote: > Duy Nguyen writes: > >>> +$ git reset -p HEAD^<1> >> >> For good practice, perhaps put "git diff --cached HEAD^" before "git commit". >> >> I tend to avoid "reset -p" and

Re: [PATCH] reset: add an example of how to split a commit into two

Duy Nguyen writes: >> +$ git reset -p HEAD^<1> > > For good practice, perhaps put "git diff --cached HEAD^" before "git commit". > > I tend to avoid "reset -p" and "checkout -p" though because sometimes > it does not work. Not sure if it's just me, I

Re: [PATCH 00/11] nd/worktree-move update

Junio C Hamano writes: > I do not recall seeing that. I however deliberately ignored another > statement because I thought it enough to answer, which was: Oops. "because I didn't think I thought it enough to answer" was what I meant.

feature request: add -q to "git branch"

It should be possible to quietly create a branch. Thanks. Kevin

[PATCH] git-parse-remote.sh: Remove op_prep argument

- Remove the third argument of error_on_missing_default_upstream that is no longer required - FIXME to remove this argument was added in commit 045fac5845 - Run "grep" on the rest of the codebase to find and remove occurences of the third argument and fix the function calls appropriately

Re: [PATCH 00/11] nd/worktree-move update

Duy Nguyen writes: > On Fri, Feb 3, 2017 at 3:21 AM, Junio C Hamano wrote: >> Johannes Schindelin writes: >> >>> Also, the more important reply was Peff's reply that suggested that the >>> proposed fix was incomplete, as it

BUG: "git checkout -q -b foo origin/foo" is not quiet

$ git checkout -q -b foo origin/foo Branch foo set up to track remote branch foo from origin. I said -q because I didn't want *any* output. I looked at the git sources, on the maint branch, and it appears to still be the case. I see this behavior on 1.7.1. Kevin

Re: [PATCH] document behavior of empty color name

Jeff King writes: > On Fri, Feb 03, 2017 at 04:24:30PM +0700, Duy Nguyen wrote: > >> > I don't think so. The default value is a git-config thing, but you would >> > want to be able to do the same thing in a config file. For example, to >> > disable coloring entirely for part of

Re: How to use git show's "%<([,trunc|ltrunc|mtrunc])"?

On 3 February 2017 at 00:06, Duy Nguyen wrote: > On Fri, Feb 3, 2017 at 12:51 AM, Hilco Wijbenga > wrote: >> Hi all, >> >> I'm trying to get the committer date printed in a custom fashion. >> Using "%cI" gets me close: >> >> $ git show --format="%cI |

Fail git pull --rebase when local merges present? (was: [ANNOUNCE] Git v2.11.1)

On Thu, Feb 2, 2017 at 6:05 PM, Junio C Hamano wrote: > * "git pull --rebase", when there is no new commits on our side since >we forked from the upstream, should be able to fast-forward without >invoking "git rebase", but it didn't. As someone who has to mentor new

Re: init --separate-git-dir does not set core.worktree

On Thu, Feb 2, 2017 at 7:37 PM, Kyle Meyer wrote: > Duy Nguyen writes: > >> On Thu, Feb 2, 2017 at 10:55 AM, Kyle Meyer wrote: >>> >>> As of 6311cfaf9 (init: do not set unnecessary core.worktree, >>> 2016-09-25), "git init --separate-git-dir"

Re: [PATCH] document behavior of empty color name

On Fri, Feb 03, 2017 at 04:24:30PM +0700, Duy Nguyen wrote: > > I don't think so. The default value is a git-config thing, but you would > > want to be able to do the same thing in a config file. For example, to > > disable coloring entirely for part of the diff, you could do: > > > > [color

Re: git-scm.com status report

On Thu, Feb 02, 2017 at 10:01:45PM +, pedro rijo wrote: > While I’m not experienced with Rails apps, I would like to give my > contribution to the Git project. I could help doing some kind of > triage, removing abusing PRs/issues (like > https://github.com/git/git-scm.com/pull/557 >

Re: git-scm.com status report

On Thu, Feb 02, 2017 at 12:54:53AM -0600, Samuel Lijin wrote: > In theory, you could also dump the build artifacts to a GH Pages repo > and host it from there, although I don't know if you would run up > against any of the usage limits[0]. The immediate problem I see with > that approach, though,

[PATCH v2 3/7] completion: improve bash completion for git-add

From: Cornelius Weig Command completion for git-add did not recognize some long-options. This commits adds completion for all long-options that are mentioned in the man-page synopsis. In addition, if the user specified `--update` or `-u`, path completion will only

[PATCH v2 4/7] completion: teach ls-remote to complete options

From: Cornelius Weig ls-remote needs to complete remote names and its own options. In addition to the existing remote name completions, do also complete the options --heads, --tags, --refs, --get-url, and --symref. Signed-off-by: Cornelius Weig

[PATCH v2 7/7] completion: recognize more long-options

From: Cornelius Weig Command completion only recognizes a subset of the available options for the various git commands. The set of recognized options needs to balance between having all useful options and to not clutter the terminal. This commit adds all long-options

[PATCH v2 0/7] completion bash: add more options and commands

From: Cornelius Weig This is the re-roll of patch series <2017015724.19360-1-cornelius.w...@tngtech.com>. This patch series adds all long-options that are mentioned in the synopsis of the man-page for the respective git-command. There are only a few exceptions,

[PATCH v2 5/7] completion: teach replace to complete options

From: Cornelius Weig Git-replace needs to complete references and its own options. In addition to the existing references completions, do also complete the options --edit --graft --format= --list --delete. Signed-off-by: Cornelius Weig

[PATCH v2 6/7] completion: teach remote subcommands to complete options

From: Cornelius Weig Git-remote needs to complete remote names, its subcommands, and options thereof. In addition to the existing subcommand and remote name completion, do also complete the options - add: --track --master --fetch --tags --no-tags --mirror= -

[PATCH v2 1/7] completion: teach submodule subcommands to complete options

From: Cornelius Weig Each submodule subcommand has specific long-options. Therefore, teach bash completion to support option completion based on the current subcommand. All long-options that are mentioned in the man-page synopsis are added. Signed-off-by: Cornelius

[PATCH v2 2/7] completion: add subcommand completion for rerere

From: Cornelius Weig Managing recorded resolutions requires command-line usage of git-rerere. Added subcommand completion for rerere and path completion for its subcommand forget. Signed-off-by: Cornelius Weig Reviewed-by: SZEDER Gábor

Re: mergetool and difftool inconsistency?

On Wed, Jan 25, 2017 at 06:58:10PM -0800, Denton Liu wrote: > Hello all, > > I was wondering if there is any reason why 'git difftool' accepts the > '-g|--gui' whereas 'git mergetool' does not have an option to accept > that flag. Please let me know if this is intentional, otherwise I can > write

Re: [PATCH] document behavior of empty color name

On Thu, Feb 02, 2017 at 01:42:44PM +0100, Jeff King wrote: > On Thu, Feb 02, 2017 at 04:16:15PM +0700, Duy Nguyen wrote: > > > > I hadn't heard anything back, > > > > Sorry I was accidentally busy during Luna new year holiday. > > No problem. That sounds much more fun than working on Git. :) >

[ANNOUNCE] Git for Windows 2.11.1

MIME-Version: 1.0 Fcc: Sent Dear Git users, It is my pleasure to announce that Git for Windows 2.11.1 is available from: https://git-for-windows.github.io/ Changes since Git for Windows v2.11.0(3) (January 14th 2017) New Features • Comes with Git v2.11.1. • Performance was

Re: [PATCH] reset: add an example of how to split a commit into two

On Fri, Feb 3, 2017 at 7:30 AM, Jacob Keller wrote: > From: Jacob Keller > > It is sometimes useful to break a commit into parts to more logically > show how the code changes. There are many possible ways to achieve this > result, but one simple

Re: [PATCH 00/11] nd/worktree-move update

On Fri, Feb 3, 2017 at 3:21 AM, Junio C Hamano wrote: > Johannes Schindelin writes: > >> Also, the more important reply was Peff's reply that suggested that the >> proposed fix was incomplete, as it misses --unpack-unreachable: >>

Re: How to use git show's "%<([,trunc|ltrunc|mtrunc])"?

On Fri, Feb 3, 2017 at 12:51 AM, Hilco Wijbenga wrote: > Hi all, > > I'm trying to get the committer date printed in a custom fashion. > Using "%cI" gets me close: > > $ git show --format="%cI | %an" master | head -n 1 > 2017-01-31T17:02:13-08:00 | Hilco Wijbenga > > I