Re: [PATCH] disable grafts during fetch/push/bundle

2014-03-04 Thread Eric Sunshine
On Tue, Mar 4, 2014 at 8:05 PM, Jeff King p...@peff.net wrote: On Tue, Mar 04, 2014 at 08:00:44PM -0500, Eric Sunshine wrote: +int commit_grafts_loaded(void) +{ + return !!commit_graft_nr; +} Did you mean !!commit_graft ? Shouldn't they produce the same results? Yes

Re: [PATCH v4 27/27] count-objects: report unused files in $GIT_DIR/repos/...

2014-03-04 Thread Eric Sunshine
On Sat, Mar 1, 2014 at 7:13 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: In linked checkouts, borrowed parts like config is taken from $GIT_COMMON_DIR. $GIT_DIR/config is never used. Report them as garbage. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com ---

Re: [PATCH] [PATCH] commit.c: Replace starts_with() with skip_prefix()

2014-03-04 Thread Eric Sunshine
On Tue, Mar 4, 2014 at 5:27 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Tue, Mar 4, 2014 at 10:54 AM, Karthik Nayak karthik@gmail.com wrote: diff --git a/commit.c b/commit.c index 6bf4fe0..71a03e3 100644 --- a/commit.c +++ b/commit.c @@ -,11 +1114,11 @@ int

Re: [PATCH v2] commit.c: Replace starts_with() with skip_prefix()

2014-03-06 Thread Eric Sunshine
On Wed, Mar 5, 2014 at 9:06 AM, Karthik Nayak karthik@gmail.com wrote: Replaces all instances of starts_with() by skip_prefix(), Use imperative mode: Replace all... which can not only be used to check presence of a prefix, but also used further on as it returns the string after the

Re: [PATCH v4 22/27] checkout: clean up half-prepared directories in --to mode

2014-03-06 Thread Eric Sunshine
On Sat, Mar 1, 2014 at 7:12 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/checkout.c | 49 +++-- 1 file changed, 47 insertions(+), 2 deletions(-) diff --git a/builtin/checkout.c

Re: [PATCH 1/6] test patch hunk editing with commit -p -m

2014-03-06 Thread Eric Sunshine
On Thu, Mar 6, 2014 at 9:50 AM, Benoit Pierre benoit.pie...@gmail.com wrote: Add (failing) test: with commit changing the environment to let hooks now that no editor will be used (by setting GIT_EDITOR to :), the edit hunk functionality does not work (no editor is launched and the whole hunk

Re: [PATCH 4/6] merge hook tests: fix and update tests

2014-03-06 Thread Eric Sunshine
On Thu, Mar 6, 2014 at 9:50 AM, Benoit Pierre benoit.pie...@gmail.com wrote: - update 'no editor' hook test and add 'editor' hook test - reset tree at the beginning of failing hook tests to avoid failures due to an unclean tree after running a previous test Signed-off-by: Benoit Pierre

[PATCH] show_ident_date: fix always-false conditional

2014-03-06 Thread Eric Sunshine
: warning: comparison of constant 9223372036854775807 with expression of type 'int' is always false [-Wtautological-constant-out-of-range-compare] if (tz == LONG_MAX || tz == LONG_MIN) Similarly for the LONG_MIN case. Fix this. Signed-off-by: Eric Sunshine sunsh

Re: [RFC/PATCH 4/4] replace: add --edit option

2014-03-06 Thread Eric Sunshine
On Thu, Mar 6, 2014 at 12:51 PM, Jeff King p...@peff.net wrote: This allows you to run: git replace --edit SHA1 to get dumped in an editor with the contents of the object for SHA1. The result is then read back in and used as a replace object for SHA1. The writing/reading is type-aware,

Re: [PATCH v4 02/27] Convert git_snpath() to strbuf_git_path()

2014-03-06 Thread Eric Sunshine
On Fri, Mar 7, 2014 at 12:03 AM, Duy Nguyen pclo...@gmail.com wrote: On Mon, Mar 3, 2014 at 7:15 AM, Duy Nguyen pclo...@gmail.com wrote: On Mon, Mar 3, 2014 at 7:02 AM, Eric Sunshine sunsh...@sunshineco.com wrote: On Sat, Mar 1, 2014 at 7:12 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote

Re: [PATCH v4 26/27] gc: support prune --repos

2014-03-06 Thread Eric Sunshine
On Sat, Mar 1, 2014 at 7:13 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/config.txt | 6 ++ builtin/gc.c | 17 + 2 files changed, 23 insertions(+) diff --git

Re: [PATH] branch.c:install_branch_config():Simplify code generating verbose message.

2014-03-07 Thread Eric Sunshine
Thanks for the submission. Some comments below to give you a taste of the Git review process... On Thu, Mar 6, 2014 at 2:58 AM, Paweł Wawruch pa...@aleg.pl wrote: From adfcfa0a334378a6242347efc0d614fa193610db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Wawruch?= pa...@aleg.pl Date:

Re: [PATCH v3] commit.c: Replace starts_with() with skip_prefix()

2014-03-07 Thread Eric Sunshine
On Thu, Mar 6, 2014 at 12:05 PM, Karthik Nayak karthik@gmail.com wrote: Replace all instances of starts_with() by skip_prefix(), which can not only be used to check presence of a prefix, but also used further on as it returns the string after the prefix, if the prefix is present. And also

Re: [PATCH] show_ident_date: fix always-false conditional

2014-03-07 Thread Eric Sunshine
On Fri, Mar 7, 2014 at 12:15 PM, Jeff King p...@peff.net wrote: On Thu, Mar 06, 2014 at 08:35:24PM -0500, Eric Sunshine wrote: 1dca155fe3fa (log: handle integer overflow in timestamps, 2014-02-24) assigns the result of strtol() to an 'int' and then checks it against LONG_MIN and LONG_MAX

Re: [PATCH] Use long for timezone in pretty.c:show_ident_date()

2014-03-07 Thread Eric Sunshine
On Fri, Mar 7, 2014 at 1:47 PM, Brian Gernhardt br...@gernhardtsoftware.com wrote: The value is parsed with strtol and compared against LONG_MIN and LONG_MAX, which doesn't make much sense for an int. Signed-off-by: Brian Gernhardt br...@gernhardtsoftware.com Thanks. Find a more complete fix

Re: [PATCH v3] commit.c: Replace starts_with() with skip_prefix()

2014-03-08 Thread Eric Sunshine
/git.github.io/blob/master/SoC-2014-Ideas.md On Fri, Mar 7, 2014 at 3:04 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Thu, Mar 6, 2014 at 12:05 PM, Karthik Nayak karthik@gmail.com wrote: Replace all instances of starts_with() by skip_prefix(), which can not only be used to check

Re: [PATCH v5 00/28] Support multiple checkouts

2014-03-09 Thread Eric Sunshine
On Fri, Mar 7, 2014 at 9:47 PM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: The diff against v4 is kinda big but it's mostly about converting `...` to $(...) and making git_path() and friends return a const string. Another notable change is I no longer attempt to support checkouts on

Re: [PATCH v5 06/28] Make git_path() aware of file relocation in $GIT_DIR

2014-03-09 Thread Eric Sunshine
On Fri, Mar 7, 2014 at 9:47 PM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: We allow the user to relocate certain paths out of $GIT_DIR via environment variables, e.g. GIT_OBJECT_DIRECTORY, GIT_INDEX_FILE and GIT_GRAFT_FILE. Callers are not supposed to use git_path() or git_pathdup() to get

Re: [PATCH v5 25/28] prune: strategies for linked checkouts

2014-03-09 Thread Eric Sunshine
On Fri, Mar 7, 2014 at 9:48 PM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: (alias R=$GIT_COMMON_DIR/repos/id) - linked checkouts are supposed to keep its location in $R/gitdir up to date. The use case is auto fixup after a manual checkout move. - linked checkouts are supposed to

Re: [PATCH v5 28/28] count-objects: report unused files in $GIT_DIR/repos/...

2014-03-09 Thread Eric Sunshine
On Fri, Mar 7, 2014 at 9:48 PM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: In linked checkouts, borrowed parts like config is taken from $GIT_COMMON_DIR. $GIT_DIR/config is never used. Report them as garbage. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- diff --git a/path.c

Re: [PATCH 1/2] status: allow NULL fmt for status_printf/status_vprintf_ln

2014-03-10 Thread Eric Sunshine
On Mon, Mar 10, 2014 at 3:27 PM, Benoit Pierre benoit.pie...@gmail.com wrote: Useful for calling status_printf only to change/reset the color (and output an additional '\n' with status_vprintf_ln). Signed-off-by: Benoit Pierre benoit.pie...@gmail.com --- wt-status.c | 3 ++- 1 file

Re: [PATCH 3/7] test patch hunk editing with commit -p -m

2014-03-10 Thread Eric Sunshine
On Mon, Mar 10, 2014 at 2:49 PM, Benoit Pierre benoit.pie...@gmail.com wrote: Add (failing) test: with commit changing the environment to let hooks now that no editor will be used (by setting GIT_EDITOR to :), the edit hunk functionality does not work (no editor is launched and the whole hunk

Re: [PATCH 6/7] merge hook tests: fix and update tests

2014-03-10 Thread Eric Sunshine
On Mon, Mar 10, 2014 at 2:49 PM, Benoit Pierre benoit.pie...@gmail.com wrote: - update 'no editor' hook test and add 'editor' hook test - make sure the tree is reset to a clean state after running a test (using test_when_finished) so later tests are not impacted As conceptually distinct

Re: [PATCH] simplified the chain if() statements of install_branch_config() function in branch.c

2014-03-10 Thread Eric Sunshine
On Mon, Mar 10, 2014 at 3:58 AM, Nemina Amarasinghe nemi...@gmail.com wrote: Nemina Amarasinghe neminaa at gmail.com writes: Sorry for the first patch. Something went wrong. This is the correct one In addition to the tautological issue pointed out by Matthieu, please note that this version of

Re: [PATCH] [GSOC] branch.c: install_branch_config: simplify if chain

2014-03-10 Thread Eric Sunshine
On Mon, Mar 10, 2014 at 1:32 AM, Adam a...@sigterm.info wrote: Simplify if chain in install_branch_config(). Nicely done. Whether the rewritten code is indeed simpler is probably a matter of taste, however, the submission itself is well executed. Signed-off-by: Adam a...@sigterm.info On this

Re: [PATCH][GSOC2014] changed logical chain in branch.c to lookup tables

2014-03-10 Thread Eric Sunshine
On Mon, Mar 10, 2014 at 3:04 PM, TamerTas tamer...@outlook.com wrote: Signed-off-by: TamerTas tamer...@outlook.com Thanks for the submission. It appears to be well executed. Read below for a concern about the approach taken. --- branch.c | 31 --- 1 file

Re: [PATCH][GSOC2014] changed logical chain in branch.c to lookup tables

2014-03-10 Thread Eric Sunshine
On Mon, Mar 10, 2014 at 5:47 PM, Tamer TAS tamer...@outlook.com wrote: Eric Sunshine wrote Even allowing internationalization of them (via N_() in the location[] and type[] tables) might not be sufficient since grammatical rules differ from language to language. I didn't fully understand

Re: [PATCH][GSoC] branch.c:install_branch_config Simplified long chain of if statements

2014-03-10 Thread Eric Sunshine
Thanks for the submission. Comments below to give you a feel for the Git review process... On Mon, Mar 10, 2014 at 6:04 PM, Vincenzo di Cicco enzodici...@gmail.com wrote: From: NaN enzodici...@gmail.com Drop this line unless it is intentionally different from your email From: header, which git

Re: [PATCH] simplified the chain if() statements of install_branch_config() function in branch.c

2014-03-11 Thread Eric Sunshine
On Tue, Mar 11, 2014 at 2:30 AM, Nemina Amarasinghe nemi...@gmail.com wrote: Eric Sunshine sunshine at sunshineco.com writes: On Mon, Mar 10, 2014 at 3:58 AM, Nemina Amarasinghe neminaa at gmail.com wrote: Nemina Amarasinghe neminaa at gmail.com writes: Sorry for the first patch

Re: [PATCH][GSoC]simplified branch.c:install_branch_config() if() statement

2014-03-11 Thread Eric Sunshine
On Tue, Mar 11, 2014 at 3:16 AM, Nemina Amarasinghe nemi...@gmail.com wrote: Subject: simplified branch.c:install_branch_config() if() statement Use imperative tone: simplify ... I hope this is the correct format for patch. Please comment on this if something is wrong. This commentary is

Re: [PATHv2] branch.c:install_branch_config():Simplify code generating verbose message.

2014-03-11 Thread Eric Sunshine
Thanks for taking review comments from your previous attempt into account. This is a more well-crafted submission. Additional comments below. On Tue, Mar 11, 2014 at 4:40 AM, Paweł Wawruch pa...@aleg.pl wrote: Subject: [PATHv2] branch.c:install_branch_config():Simplify code generating verbose

Re: [PATCH][GSOC2014] changed logical chain in branch.c to lookup tables

2014-03-11 Thread Eric Sunshine
, for instance, you might write: Subject: install_branch_config: change logical chain to lookup table Eric Sunshine wrote On Mon, Mar 10, 2014 at 5:47 PM, Tamer TAS lt; tamertas@ gt; wrote: Section 4.3 of the GNU gettext manual [1] explains the issues in more detail. I urge you to read

[PATCH] sh-i18n--envsubst: retire unused string_list_member()

2014-03-11 Thread Eric Sunshine
This static function has no callers, nor has it had any since its introduction in ba67aaf2d05d (git-sh-i18n--envsubst: our own envsubst(1) for eval_gettext(), 2011-05-14). Remove it. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- The latest Apple developer tools (just released) has

Re: [PATCH v4] install_branch_config: simplify verbose messages logic

2014-03-11 Thread Eric Sunshine
On Tue, Mar 11, 2014 at 8:33 PM, Paweł Wawruch pa...@aleg.pl wrote: Replace the chain of if statements with table of strings. Signed-off-by: Paweł Wawruch pa...@aleg.pl --- The changes proposed by Junio C Hamano: Improvement of indentations. Removed an unused variable. Better, thanks. More

Re: [PATCH] submodule: add verbose mode for add/update

2014-03-12 Thread Eric Sunshine
On Mar 12, 2014, at 2:38 AM, Orgad Shaneh org...@gmail.com wrote: Executes checkout without -q — Missing sign-off. See Documentation/SubmittingPatches. Your patch is badly whitespace-damaged, as if it was pasted into your email client. “git send-email” can avoid this problem. As I’m not a

Re: [PATCH] SoC 2014 MicroProject No.8:change multiple if-else statement to table-driven approach

2014-03-12 Thread Eric Sunshine
Thanks for the submission. Comments below to give you a taste of the Git review process... On Tue, Mar 11, 2014 at 11:47 PM, Yao Zhao zhaox...@umn.edu wrote: Subject: SoC 2014 MicroProject No.8:change multiple if-else statement to table-driven approach The email subject is extracted

Re: Re* [RFC PATCH 2/1] Make request-pull able to take a refspec of form local:remote

2014-03-12 Thread Eric Sunshine
On Wed, Mar 12, 2014 at 2:04 PM, Junio C Hamano gits...@pobox.com wrote: Subject: [PATCH] request-pull: documentation updates The original description talked only about what it does. Instead, start it with the purpose of the command, i.e. what it is used for, and then mention what it does to

Re: [PATCH v4] install_branch_config: simplify verbose messages logic

2014-03-13 Thread Eric Sunshine
On Thu, Mar 13, 2014 at 2:34 PM, Junio C Hamano gits...@pobox.com wrote: Eric Sunshine sunsh...@sunshineco.com writes: Shouldn't this logic [to decide what the printf arguments should be] also be encoded in the table? ... The same argument also applies to computation of the 'name' variable

Re: [PATCH][GSOC2014] install_branch_config: change logical chain to lookup table

2014-03-13 Thread Eric Sunshine
On Wed, Mar 12, 2014 at 5:24 PM, TamerTas tamer...@outlook.com wrote: Signed-off-by: TamerTas tamer...@outlook.com -- There should be three hyphens --- here but somehow you have only two --. Since --- is detected automatically when the patch is applied, this deviation can be problematic.

Re: [PATCH v5] install_branch_config: simplify verbose messages logic

2014-03-13 Thread Eric Sunshine
On Wed, Mar 12, 2014 at 7:47 PM, Paweł Wawruch pa...@aleg.pl wrote: Replace the chain of if statements with table of strings. Signed-off-by: Paweł Wawruch pa...@aleg.pl --- Thanks to Eric Sunshine and Junio C Hamano. Simplified printing logic. The name moved to a table. v4: http

Re: [PATCH v4] install_branch_config: simplify verbose messages logic

2014-03-13 Thread Eric Sunshine
On Thu, Mar 13, 2014 at 4:35 PM, Eric Sunshine sunsh...@sunshineco.com wrote: A more table-driven approach might look something like this: struct M { const char *s; const char **a1; const char **a2; } message[][2][2] = {{{ { Branch %s set ... %s ... %s, shortname, origin

Re: [PATCH] GSoC Change multiple if-else statements to be table-driven

2014-03-13 Thread Eric Sunshine
Thanks for the resubmission. Comments below. On Thu, Mar 13, 2014 at 4:20 PM, Yao Zhao zhaox...@umn.edu wrote: Subject: [PATCH] GSoC Change multiple if-else statements to be table-driven It's a good idea to let reviewers know that this is attempt 2. Do so by saying [PATCH v2]. Your next one

Re: [PATCH] GSoC Change multiple if-else statements to be table-driven

2014-03-17 Thread Eric Sunshine
On Fri, Mar 14, 2014 at 12:54 PM, Junio C Hamano gits...@pobox.com wrote: Eric Sunshine sunsh...@sunshineco.com writes: On Thu, Mar 13, 2014 at 4:20 PM, Yao Zhao zhaox...@umn.edu wrote: Subject: [PATCH] GSoC Change multiple if-else statements to be table-driven It's a good idea to let

Re: [PATCH][GSOC2014] install_branch_config: change logical chain to lookup table

2014-03-17 Thread Eric Sunshine
On Fri, Mar 14, 2014 at 5:30 PM, TamerTas tamer...@outlook.com wrote: Signed-off-by: TamerTas tamer...@outlook.com --- Thanks again for the feedback it's been a great learning experience. Comments Below :) I have refactored the commit [1] to * suggested changes [2]. format-patch was

Re: [PATCH/GSoC_v3] branch.c: turn nested if-else logic to table-driven

2014-03-17 Thread Eric Sunshine
On Sun, Mar 16, 2014 at 2:08 AM, Yao Zhao zhaox...@umn.edu wrote: Signed-off-by: Yao Zhao zhaox...@umn.edu --- branch.c | 53 + 1 file changed, 29 insertions(+), 24 deletions(-) Hello Eric, Thank you and Junio for reviewing my code. It is

Re: [PATCH] branch.c: simplify chain of if statements

2014-03-17 Thread Eric Sunshine
On Mon, Mar 17, 2014 at 3:23 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Dragos Foianu dragos.foi...@gmail.com writes: + const char *verbose_prints[4] = { + Branch %s set up to track remote branch %s from %s%s, + Branch %s set up to track local branch

Re: [PATCH] branch.c: simplify chain of if statements

2014-03-17 Thread Eric Sunshine
Thanks for the submission. Comments below to give you a taste of the Git review process... On Sun, Mar 16, 2014 at 5:22 PM, Dragos Foianu dragos.foi...@gmail.com wrote: This patch uses a table-driven approach in order to make the code cleaner. In fact, this change is not table-driven (emphasis

Re: [PATCH] Rewrite the diff-no-index.c

2014-03-17 Thread Eric Sunshine
Thanks for the resubmission. Comments below... On Sun, Mar 16, 2014 at 8:44 AM, ubuntu2...@126.com wrote: From: 沈承恩 ubuntu2...@126.com Subject: [PATCH] Rewrite the diff-no-index.c This is your second version of the patch, so you should say [PATCH v2]. Most patches rewrite something, so

Re: [PATCH] add: Use struct argv_array in run_add_interactive()

2014-03-17 Thread Eric Sunshine
On Sun, Mar 16, 2014 at 7:42 AM, Thomas Rast t...@thomasrast.ch wrote: Fabian Ruch baf...@gmail.com writes: run_add_interactive() in builtin/add.c manually computes array bounds and allocates a static args array to build the add--interactive command line, which is error-prone. Use the

Re: [PATCH] Removed subshell invocations in many of the tests when possible

2014-03-17 Thread Eric Sunshine
Thanks for the submission. Comments below to give you a taste of the Git review process.. On Sun, Mar 16, 2014 at 3:40 AM, David Tran unsignedz...@gmail.com wrote: Subject: Removed subshell invocations in many of the tests when possible Use imperative mode: remove rather than removed many of

Re: [PATCH][GSOC] Selection of the verbose message is replaced with generated message in install_branch_config()

2014-03-17 Thread Eric Sunshine
Thanks for the submission. Comments below to give you a taste of the Git review process... On Mon, Mar 17, 2014 at 5:55 AM, Aleksey Mokhovikov moxobu...@gmail.com wrote: Subject: [GSOC] Selection of the verbose message is replaced with generated message in install_branch_config() Mentioning

Re: [PATCH] mv: prevent mismatched data when ignoring errors.

2014-03-17 Thread Eric Sunshine
On Mon, Mar 17, 2014 at 11:07 AM, Michael Haggerty mhag...@alum.mit.edu wrote: On 03/17/2014 07:33 AM, Junio C Hamano wrote: Junio C Hamano gits...@pobox.com writes: Would it make sense to go one step further to introduce two macros to make this kind of screw-up less likely? potential

Re: [PATCH v2] log: add --nonlinear-barrier to help see non-linear history

2014-03-17 Thread Eric Sunshine
On Mon, Mar 17, 2014 at 8:51 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- v2 renames the option name to --nonlinear-barrier and fixes using it with --dense. Best used with --no-merges to see patch series.

Re: [PATCH] GSoC Change multiple if-else statements to be table-driven

2014-03-17 Thread Eric Sunshine
On Mon, Mar 17, 2014 at 10:11 AM, Michael Haggerty mhag...@alum.mit.edu wrote: On 03/17/2014 08:31 AM, Junio C Hamano wrote: Eric Sunshine sunsh...@sunshineco.com writes: Perhaps it is time to mark this microproject as taken on the GSoC page [2], along a fews others for which we have received

Re: [PATCH 3/7] test patch hunk editing with commit -p -m

2014-03-17 Thread Eric Sunshine
On Mon, Mar 17, 2014 at 3:46 PM, Benoit Pierre benoit.pie...@gmail.com wrote: On Mon, Mar 17, 2014 at 7:49 PM, Junio C Hamano gits...@pobox.com wrote: Benoit Pierre benoit.pie...@gmail.com writes: Add (failing) tests: with commit changing the environment to let hooks know that no editor will

Re: [PATCH] subtree: initialize prefix variable

2014-03-17 Thread Eric Sunshine
On Mon, Mar 17, 2014 at 3:59 PM, Jeff King p...@peff.net wrote: On Mon, Mar 17, 2014 at 01:58:00PM +0100, Gilles Filippini wrote: Test 21 from contrib/subtree/t/t7900-subtree.sh fails when an environment variable 'prefix' is set. For instance here is what happens when prefix=/usr: I think

Re: [PATCH 1/1] general style: replaces memcmp() with starts_with()

2014-03-17 Thread Eric Sunshine
On Mon, Mar 17, 2014 at 7:46 PM, Quint Guvernator quintus.pub...@gmail.com wrote: 2014-03-17 18:52 GMT-04:00 Junio C Hamano gits...@pobox.com: Thanks. This probably needs retitled, though (hint: replaces? who does so?) and the message rewritten (see numerous reviews on other GSoC micros from

Re: [PATCH 1/1] general style: replaces memcmp() with starts_with()

2014-03-17 Thread Eric Sunshine
On Mon, Mar 17, 2014 at 10:33 PM, Quint Guvernator quintus.pub...@gmail.com wrote: 2014-03-17 21:59 GMT-04:00 Eric Sunshine sunsh...@sunshineco.com: I can't speak for Junio, but the description could be made more concise and to-the-point. Aside from using imperative voice, you can eliminate

Re: [GSoC14][RFC] Is there any interest in adding a port of checkpatch.pl to contrib/?

2014-03-17 Thread Eric Sunshine
On Mon, Mar 17, 2014 at 9:38 PM, Jacopo Notarstefano jacopo.notarstef...@gmail.com wrote: It seems to me that the topic of adding the checkpatch.pl script to Git's source tree has cropped up several times in the past, as recently as a couple of days ago: $gmane/243607. It should be noted that

Re: [PATCH v2] tests: set temp variables using 'env' in test function instead of subshell

2014-03-18 Thread Eric Sunshine
On Tue, Mar 18, 2014 at 8:08 AM, David Tran unsignedz...@gmail.com wrote: Originally, the code used subshells instead of FOO=BAR command because the variable would otherwise leak into the surrounding context of the POSIX shell when 'command' is a shell function. The subshell was used to hold

Re: [PATCH] branch.c: simplify chain of if statements

2014-03-18 Thread Eric Sunshine
On Mon, Mar 17, 2014 at 7:46 AM, Dragos Foianu dragos.foi...@gmail.com wrote: Eric Sunshine sunshine at sunshineco.com writes: Matthieu already mentioned [2] that this sort of lego string construction is not internationalization-friendly. See section 4.3 [3] of the gettext manual for details

Re: [PATCHv2] branch.c: simplify chain of if statements

2014-03-18 Thread Eric Sunshine
Thanks for the resubmission. Comments below... On Mon, Mar 17, 2014 at 11:51 AM, Dragos Foianu dragos.foi...@gmail.com wrote: This patch uses a table to store the different messages that can be emitted by the verbose install_branch_config function. It computes an index based on the three flags

Re: [PATCH v2] tests: set temp variables using 'env' in test function instead of subshell

2014-03-18 Thread Eric Sunshine
On Tue, Mar 18, 2014 at 5:45 PM, Jeff King p...@peff.net wrote: On Tue, Mar 18, 2014 at 01:37:39PM -0700, Junio C Hamano wrote: diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh index c9c426c..3e3f77b 100755 --- a/t/t1300-repo-config.sh +++ b/t/t1300-repo-config.sh @@

Re: [PATCH] fsck.c:fsck_commit() use starts_with() and skip_prefix()

2014-03-18 Thread Eric Sunshine
Thanks for the submission. Comments below to give you a taste of the Git review process... On Tue, Mar 18, 2014 at 6:41 PM, Othman Darraz darraz...@gmail.com wrote: use of starts_with() instead of memcmp() use of skip_prefix instead of memcmp() and strlen() Write proper sentences to explain

Re: [PATCH] fsck.c:fsck_commit() use starts_with() and skip_prefix()

2014-03-18 Thread Eric Sunshine
On Tue, Mar 18, 2014 at 7:09 PM, Eric Sunshine sunsh...@sunshineco.com wrote: diff --git a/fsck.c b/fsck.c index 64bf279..5eae856 100644 --- a/fsck.c +++ b/fsck.c @@ -290,7 +290,7 @@ static int fsck_commit(struct commit *commit, fsck_error error_func) int parents = 0; int

Re: [PATCHv2] branch.c: simplify chain of if statements

2014-03-18 Thread Eric Sunshine
On Tue, Mar 18, 2014 at 6:31 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Mon, Mar 17, 2014 at 11:51 AM, Dragos Foianu dragos.foi...@gmail.com wrote: This patch uses a table to store the different messages that can be emitted by the verbose install_branch_config function. It computes

Re: [PATCH] [GSoC 2014]diff: Imported dir.h and renamed read_directory()

2014-03-18 Thread Eric Sunshine
Thanks for the submission. Comments below to give you a taste of the Git review process... On Tue, Mar 18, 2014 at 8:35 PM, Brian Bourn ba.bo...@gmail.com wrote: Subject: diff: Imported dir.h and renamed read_directory() Use imperative voice: import dir.h and rename read_directory() this was

Re: [PATCH 2/2][GSoC 2014] diff: used is_dot_or_dotdot() in code

2014-03-18 Thread Eric Sunshine
On Tue, Mar 18, 2014 at 9:30 PM, babourn ba.bo...@gmail.com wrote: Subject: diff: used is_dot_or_dotdot() in code Use imperative voice: use rather than used in accordance with the GSoC Microproject implemented This commentary will not have much meaning to someone reading the commit log months

Re: [PATCH v3] tests: use env to run commands with temporary env-var settings

2014-03-18 Thread Eric Sunshine
On Tue, Mar 18, 2014 at 2:54 PM, David Tran unsignedz...@gmail.com wrote: Originally, we would use VAR=VAL command to execute a test command with environment variable(s) only for that command. This does not work for commands that are shell functions (most notably test functions like

Re: [PATCH 2/2][GSoC 2014] diff: used is_dot_or_dotdot() in code

2014-03-18 Thread Eric Sunshine
On Tue, Mar 18, 2014 at 11:58 PM, Brian Bourn ba.bo...@gmail.com wrote: On Tue, Mar 18, 2014 at 11:45 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Tue, Mar 18, 2014 at 9:30 PM, babourn ba.bo...@gmail.com wrote: Subject: diff: used is_dot_or_dotdot() in code Signed-off-by: Brian Bourn

Re: [PATCH v5] use starts_with() instead of !memcmp()

2014-03-18 Thread Eric Sunshine
On Tue, Mar 18, 2014 at 9:18 PM, Quint Guvernator quintus.pub...@gmail.com wrote: Another version, this time very in line with the review and commentary of Junio, Eric, and Michael. This version boasts a revamped commit message and fewer but surer hunks changed. Explaining what changed in

Re: [PATCH v2 1/2] [GSoC] diff: rename read_directory()

2014-03-19 Thread Eric Sunshine
On Wed, Mar 19, 2014 at 2:29 AM, Brian Bourn ba.bo...@gmail.com wrote: Subject: diff: rename read_directory() I think you mean diff-no-index rather than diff. From: Brian Bourn ba.bo...@gmail.com Unless this is intentionally different from the address from which you sent the email, you

Re: [PATCH v2 2/2] [GSoC] diff:use is_dot_or_dotdot() in code

2014-03-19 Thread Eric Sunshine
On Wed, Mar 19, 2014 at 2:29 AM, Brian Bourn ba.bo...@gmail.com wrote: Subject: diff:use is_dot_or_dotdot() in code Wrong subject. See below. From: Brian Bourn ba.bo...@gmail.com Drop this. git am will grab your name and address automatically from the email header when applying the patch.

Re: [PATCH][GSOC] Selection of the verbose message is replaced with generated message in install_branch_config()

2014-03-19 Thread Eric Sunshine
Thanks for the resubmission. Comments below... On Tue, Mar 18, 2014 at 10:33 AM, Aleksey Mokhovikov moxobu...@gmail.com wrote: Subject: [PATCH][GSOC] Selection of the verbose message is replaced with generated message in install_branch_config() Say [PATCH v2] to indicate your second attempt.

Re: [PATCH] fsck.c:fsck_commit() use starts_with() and skip_prefix()

2014-03-19 Thread Eric Sunshine
On Wed, Mar 19, 2014 at 5:04 AM, Othman Darraz darraz...@gmail.com wrote: 2014-03-19 0:12 GMT+01:00 Eric Sunshine sunsh...@sunshineco.com: On Tue, Mar 18, 2014 at 7:09 PM, Eric Sunshine sunsh...@sunshineco.com wrote: diff --git a/fsck.c b/fsck.c index 64bf279..5eae856 100644 --- a/fsck.c

Re: [PATCH] diff-no-index.c: rewrote read_directory() to use is_dot_or_dotdot() function.

2014-03-19 Thread Eric Sunshine
Thanks for the submission. Comments below to give you a taste of the Git review process... On Wed, Mar 19, 2014 at 4:31 AM, Andrei Dinu mandrei.d...@gmail.com wrote: Subject: diff-no-index.c: rewrote read_directory() to use is_dot_or_dotdot() function. Use imperative tone: rewrite instead of

Re: [PATCH 1/3][GSOC] diff: rename read_directory() to get_directory_list()

2014-03-19 Thread Eric Sunshine
On Mon, Mar 17, 2014 at 5:30 PM, Hiroyuki Sano sh19910...@gmail.com wrote: Including dir.h in diff-no-index.c, it causes a compile error, because the same name function read_directory() is declared globally in dir.h. This change is to avoid conflicts as above. This explanation is slightly

Re: [PATCH v3 1/2][GSoC] diff-no-index: rename read_directory()

2014-03-19 Thread Eric Sunshine
On Wed, Mar 19, 2014 at 2:31 PM, Junio C Hamano gits...@pobox.com wrote: Brian Bourn ba.bo...@gmail.com writes: It would be desirable to replace manual checking of . or .. in diff-no-index.c with is_dot_or_dotdot(), which is defined in dir.h, however, dir.h declares a read_directory() which

Re: [PATCH v2 1/3][GSOC] diff: rename read_directory() to get_path_list()

2014-03-19 Thread Eric Sunshine
On Wed, Mar 19, 2014 at 7:23 AM, Hiroyuki Sano sh19910...@gmail.com wrote: Subject: diff: rename read_directory() to get_path_list() You probably mean 'diff-no-index' here rather than 'diff'. Including dir.h in diff-no-index.c, it causes a compile error, because the same name function

Re: [PATCH v2 2/3][GSOC] diff: use is_dot_or_dotdot() instead of strcmp()

2014-03-19 Thread Eric Sunshine
On Wed, Mar 19, 2014 at 7:23 AM, Hiroyuki Sano sh19910...@gmail.com wrote: Subject: diff: use is_dot_or_dotdot() instead of strcmp() You probably meant 'diff-no-index' rather than 'diff'. You could make the subject a bit more explanatory by saying: use is_dot_or_dotdot() instead of a

Re: [PATCH v3] rev-parse --parseopt: option argument name hints

2014-03-20 Thread Eric Sunshine
On Thu, Mar 20, 2014 at 4:44 AM, Ilya Bobyr ilya.bo...@gmail.com wrote: Built-in commands can specify names for option arguments when usage text is generated for a command. sh based commands should be able to do the same. Option argument name hint is any text that comes after [*=?!] after

Re: [GSoc PATCH 1/3] diff-no-index.c: rename read_directory()

2014-03-20 Thread Eric Sunshine
On Thu, Mar 20, 2014 at 3:52 PM, Andrei Dinu mandrei.d...@gmail.com wrote: Avoid the conflict between read_directory() from diff-no-index.c and read_directory() from dir.h This message is lacking a bit of information. Since diff-no-index.c does not presently include dir.h, the reader is left to

Re: [PATCHv2] fsck.c:fsck_commit() use skip_prefix() and starts_with()

2014-03-20 Thread Eric Sunshine
On Wed, Mar 19, 2014 at 9:44 AM, Othman Darraz darraz...@gmail.com wrote: Subject: fsck.c:fsck_commit() use skip_prefix() and starts_with() Add a space after the colon. Add a colon after fsck_commit(). make buffer const char* because the content of the memory referenced by this variable

Re: [PATCHv2] branch.c: simplify chain of if statements

2014-03-20 Thread Eric Sunshine
On Wed, Mar 19, 2014 at 7:12 PM, Dragos Foianu dragos.foi...@gmail.com wrote: Eric Sunshine sunshine at sunshineco.com writes: On Tue, Mar 18, 2014 at 6:31 PM, Eric Sunshine sunshine at sunshineco.com wrote: One other observation: You have a one-off error in your out-of-bounds check

Re: [PATCHv2] branch.c: simplify chain of if statements

2014-03-20 Thread Eric Sunshine
On Thu, Mar 20, 2014 at 8:40 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Wed, Mar 19, 2014 at 7:12 PM, Dragos Foianu dragos.foi...@gmail.com wrote: Eric Sunshine sunshine at sunshineco.com writes: On Tue, Mar 18, 2014 at 6:31 PM, Eric Sunshine sunshine at sunshineco.com wrote: One

Re: [PATCH] builtin/apply.c: use iswspace() to detect line-ending-like chars

2014-03-20 Thread Eric Sunshine
On Thu, Mar 20, 2014 at 3:39 PM, George Papanikolaou g3orge@gmail.com wrote: Removing the bloat of checking for both '\r' and '\n' with the prettier iswspace() function which checks for other characters as well. (read: \f \t \v) Use imperative mood. Remove rather than Removing. Bloat?

Re: [PATCH] GSoC Miniproject 15. Rewrite fsck.c:fsck_commit()

2014-03-20 Thread Eric Sunshine
Thanks for the submission. Comments below to give you a feel for the Git review process... On Thu, Mar 20, 2014 at 9:54 PM, Ashwin Jha ajha@gmail.com wrote: Subject: [PATCH] GSoC Miniproject 15. Rewrite fsck.c:fsck_commit() The subject becomes part of the permanent Git history, but the fact

Re: [PATCH][GSOC] Selection of the verbose message is replaced with generated message in install_branch_config()

2014-03-20 Thread Eric Sunshine
On Thu, Mar 20, 2014 at 7:56 AM, Aleksey Mokhovikov moxobu...@gmail.com wrote: On 03/19/2014 04:21 PM, Eric Sunshine wrote: Thanks for the resubmission. Comments below... On Tue, Mar 18, 2014 at 10:33 AM, Aleksey Mokhovikov moxobu...@gmail.com wrote: Subject: [PATCH][GSOC] Selection

Re: [PATCH] Rewritten fsck.c:fsck_commit() through using starts_with() instead of memcmp()

2014-03-20 Thread Eric Sunshine
Thanks for the submission. Comments below to give you a feel for the Git review process... On Thu, Mar 20, 2014 at 1:48 AM, blacksimit cengoguzhanu...@gmail.com wrote: Subject: Rewritten fsck.c:fsck_commit() through using starts_with() instead of memcmp() Use imperative mood (rewrite instead

Re: [PATCH] Rewritten fetch-pack.c:filter_refs() using starts_with() instead of memcmp()

2014-03-21 Thread Eric Sunshine
Thanks for the submission. Comments below to give you a taste of the Git review process... On Thu, Mar 20, 2014 at 6:04 AM, MustafaOrkunAcar mustafaorkuna...@gmail.com wrote: Subject: Rewritten fetch-pack.c:filter_refs() using starts_with() instead of memcmp() Use imperative mood: Rewrite

Re: [GUILT 20/28] guilt graph: Handle patch names containing quotes.

2014-03-21 Thread Eric Sunshine
On Fri, Mar 21, 2014 at 3:31 AM, Per Cederqvist ced...@opera.com wrote: Quote quotes with a backslash in the guitl graph output. Otherwise, s/guitl/guilt/ the dot file could contain syntax errors. Added a test case. --- guilt-graph | 2 ++ regression/t-033.out | 22

Re: [PATCH v5] use starts_with() instead of !memcmp()

2014-03-21 Thread Eric Sunshine
On Fri, Mar 21, 2014 at 12:53 PM, Junio C Hamano gits...@pobox.com wrote: Eric Sunshine sunsh...@sunshineco.com writes: On Tue, Mar 18, 2014 at 9:18 PM, Quint Guvernator quintus.pub...@gmail.com wrote: Another version, this time very in line with the review and commentary of Junio, Eric

Re: [PATCH] GSoC Miniproject 15. Rewrite fsck.c:fsck_commit()

2014-03-21 Thread Eric Sunshine
On Fri, Mar 21, 2014 at 5:18 AM, Ashwin Jha ajha@gmail.com wrote: On Fri, Mar 21, 2014 at 9:03 AM, Eric Sunshine sunsh...@sunshineco.com wrote: On Thu, Mar 20, 2014 at 9:54 PM, Ashwin Jha ajha@gmail.com wrote: Subject: [PATCH] GSoC Miniproject 15. Rewrite fsck.c:fsck_commit

Re: [PATCH 1/2] diff-no-index.c: rename read_directory()

2014-03-21 Thread Eric Sunshine
On Fri, Mar 21, 2014 at 2:56 PM, Andrei Dinu mandrei.d...@gmail.com wrote: Subject: [PATCH 1/2] diff-no-index.c: rename read_directory() It is helpful to reviewers if you indicate that this is a resubmission by placing 'vN' inside [...], where N is the reroll number. For instance, if this is

Re: [PATCH v2] Rewrite fsck.c:fsck_commit() replace memcmp() with starts_with()

2014-03-21 Thread Eric Sunshine
In addition to the valuable review comments by Junio and Matthieu, see a few more below... On Fri, Mar 21, 2014 at 12:37 PM, blacksimit cengoguzhanu...@gmail.com wrote: From: Oguzhan Unlu cengoguzhanu...@gmail.com My solution to make lines containing buffer += a_number; clearer to anyone is

Re: [PATCH 11/12] t0001: drop useless subshells

2014-03-21 Thread Eric Sunshine
On Thu, Mar 20, 2014 at 7:21 PM, Jeff King p...@peff.net wrote: Many tests use subshells, but don't actually change the shell environment. They were probably cargo-culted from earlier tests which did need subshells. Drop the useless ones. Signed-off-by: Jeff King p...@peff.net --- These

Re: [PATCH][GSOC] Selection of the verbose message is replaced with generated message in install_branch_config()

2014-03-21 Thread Eric Sunshine
On Fri, Mar 21, 2014 at 5:13 PM, Michael Haggerty mhag...@alum.mit.edu wrote: On 03/21/2014 06:09 PM, Junio C Hamano wrote: Eric Sunshine sunsh...@sunshineco.com writes: Sorry, you're right about message[0] case not being a crasher (though the assert() still seems overkill). Assert() often

Re: [PATCH] builtin/apply.c: use iswspace() to detect line-ending-like chars

2014-03-21 Thread Eric Sunshine
[Please reply on-list to review comments. Other people may learn from the discussion or have comments of their own.] On Fri, Mar 21, 2014 at 6:00 PM, George Papanikolaou g3orge@gmail.com wrote: On Fri, Mar 21, 2014 at 4:48 AM, Eric Sunshine sunsh...@sunshineco.com wrote: Did you verify

Re: [PATCH] Modify fsck_commit. Replace memcmp by skip_prefix

2014-03-23 Thread Eric Sunshine
Thanks for the resubmission. Comments below... On Sat, Mar 22, 2014 at 11:12 AM, Ashwin Jha ajha@gmail.com wrote: Subject: [PATCH] Modify fsck_commit. Replace memcmp by skip_prefix In his review, Tanay already mentioned indicating a resubmission via [PATCH vN]. Additionally, specify the

Re: [PATCH v2] Rewrite strbuf.c:strbuf_cmp() replace memcmp() with starts_with()

2014-03-23 Thread Eric Sunshine
In addition to the valuable review comments already provided by Alexandru and David, see a few more below. On Sat, Mar 22, 2014 at 5:25 PM, Mustafa Orkun Acar mustafaorkuna...@gmail.com wrote: Subject: [PATCH v2] Rewrite strbuf.c:strbuf_cmp() replace memcmp() with starts_with() This isn't

<    1   2   3   4   5   6   7   8   9   10   >