* [PATCH] SoC 2014 MicroProject No.8:change multiple if-else statement to table-driven approach @ 2014-03-12 3:47 Yao Zhao 2014-03-12 9:21 ` Eric Sunshine 0 siblings, 1 reply; 13+ messages in thread From: Yao Zhao @ 2014-03-12 3:47 UTC (permalink / raw) To: git; +Cc: Yao Zhao Signed-off-by: Yao Zhao <zhaox383@umn.edu> --- branch.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 53 insertions(+), 2 deletions(-) diff --git a/branch.c b/branch.c index 723a36b..6432e27 100644 --- a/branch.c +++ b/branch.c @@ -53,7 +53,20 @@ void install_branch_config(int flag, const char *local, const char *origin, cons int remote_is_branch = starts_with(remote, "refs/heads/"); struct strbuf key = STRBUF_INIT; int rebasing = should_setup_rebase(origin); - + char** print_list = malloc(8 * sizeof(char*)); + char* arg1=NULL; + char* arg2=NULL; + char* arg3=NULL; + int index=0; + + print_list[7] = _("Branch %s set up to track remote branch %s from %s by rebasing."); + print_list[6] = _("Branch %s set up to track remote branch %s from %s."); + print_list[5] = _("Branch %s set up to track local branch %s by rebasing."); + print_list[4] = _("Branch %s set up to track local branch %s."); + print_list[3] = _("Branch %s set up to track remote ref %s by rebasing."); + print_list[2] = _("Branch %s set up to track remote ref %s."); + print_list[1] = _("Branch %s set up to track local ref %s by rebasing."); + print_list[0] = _("Branch %s set up to track local ref %s."); if (remote_is_branch && !strcmp(local, shortname) && !origin) { @@ -77,7 +90,44 @@ void install_branch_config(int flag, const char *local, const char *origin, cons strbuf_release(&key); if (flag & BRANCH_CONFIG_VERBOSE) { - if (remote_is_branch && origin) + if(remote_is_branch) + index += 4; + if(origin) + index += 2; + if(rebasing) + index += 1; + + if(index < 0 || index > 7) + { + die("BUG: impossible combination of %d and %p", + remote_is_branch, origin); + } + + if(index <= 4) { + arg1 = local; + arg2 = remote; + } + else if(index > 6) { + arg1 = local; + arg2 = shortname; + arg3 = origin; + } + else { + arg1 = local; + arg2 = shortname; + } + + if(!arg3) { + printf_ln(print_list[index],arg1,arg2); + } + else { + printf_ln(print_list[index],arg1,arg2,arg3); + } + + free(print_list); + + +/* if (remote_is_branch && origin) printf_ln(rebasing ? _("Branch %s set up to track remote branch %s from %s by rebasing.") : _("Branch %s set up to track remote branch %s from %s."), @@ -100,6 +150,7 @@ void install_branch_config(int flag, const char *local, const char *origin, cons else die("BUG: impossible combination of %d and %p", remote_is_branch, origin); +*/ } } -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] SoC 2014 MicroProject No.8:change multiple if-else statement to table-driven approach 2014-03-12 3:47 [PATCH] SoC 2014 MicroProject No.8:change multiple if-else statement to table-driven approach Yao Zhao @ 2014-03-12 9:21 ` Eric Sunshine 2014-03-13 20:20 ` [PATCH] GSoC Change multiple if-else statements to be table-driven Yao Zhao 0 siblings, 1 reply; 13+ messages in thread From: Eric Sunshine @ 2014-03-12 9:21 UTC (permalink / raw) To: Yao Zhao; +Cc: Git List 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 <zhaox383@umn.edu> wrote: > Subject: SoC 2014 MicroProject No.8:change multiple if-else statement to table-driven approach The email subject is extracted automatically by "git am" as the first line of the patch's commit message so it should contain only text which is relevant to the commit message. In this case, everything before "changes" is merely commentary for readers of the email, and not relevant to the commit message. It is indeed a good idea to let reviewers know that this submission is for GSoC, and you can indicate this as such: Subject: [PATCH GSoC] change multiple if-else statements to be table-driven > Signed-off-by: Yao Zhao <zhaox383@umn.edu> > --- The additional information that this is GSoC microproject #8 would go in the "commentary" area right here after the "---" following your sign-off. > branch.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 53 insertions(+), 2 deletions(-) The patch is rife with style violations. I'll point out the first instance of each violation, but do be sure to fix all remaining ones when you resubmit. See Documentation/CodingGuidelines for details. > diff --git a/branch.c b/branch.c > index 723a36b..6432e27 100644 > --- a/branch.c > +++ b/branch.c > @@ -53,7 +53,20 @@ void install_branch_config(int flag, const char *local, const char *origin, cons > int remote_is_branch = starts_with(remote, "refs/heads/"); > struct strbuf key = STRBUF_INIT; > int rebasing = should_setup_rebase(origin); > - > + char** print_list = malloc(8 * sizeof(char*)); Style: char **print_list Why allocate 'print_list' on the heap? An automatic variable 'char const *print_list[]' would be more idiomatic and less likely to be leaked. In fact, your heap-allocated 'print_list' _is_ being leaked a few lines down when the function returns early after warning that a branch can not be its own upstream. > + char* arg1=NULL; > + char* arg2=NULL; > + char* arg3=NULL; Style: char *var Style: whitespace: var = NULL > + int index=0; > + > + print_list[7] = _("Branch %s set up to track remote branch %s from %s by rebasing."); > + print_list[6] = _("Branch %s set up to track remote branch %s from %s."); > + print_list[5] = _("Branch %s set up to track local branch %s by rebasing."); > + print_list[4] = _("Branch %s set up to track local branch %s."); > + print_list[3] = _("Branch %s set up to track remote ref %s by rebasing."); > + print_list[2] = _("Branch %s set up to track remote ref %s."); > + print_list[1] = _("Branch %s set up to track local ref %s by rebasing."); > + print_list[0] = _("Branch %s set up to track local ref %s."); If you make print_list[] an automatic variable, then you can declare and populate it via a simple initializer. No need for this manual approach. > if (remote_is_branch > && !strcmp(local, shortname) > && !origin) { > @@ -77,7 +90,44 @@ void install_branch_config(int flag, const char *local, const char *origin, cons > strbuf_release(&key); > > if (flag & BRANCH_CONFIG_VERBOSE) { > - if (remote_is_branch && origin) > + if(remote_is_branch) Style: whitespace: if (...) > + index += 4; > + if(origin) > + index += 2; > + if(rebasing) > + index += 1; > + > + if(index < 0 || index > 7) > + { > + die("BUG: impossible combination of %d and %p", > + remote_is_branch, origin); > + } > + > + if(index <= 4) { > + arg1 = local; > + arg2 = remote; > + } > + else if(index > 6) { Style: } else if (...) { > + arg1 = local; > + arg2 = shortname; > + arg3 = origin; > + } > + else { > + arg1 = local; > + arg2 = shortname; > + } > + > + if(!arg3) { > + printf_ln(print_list[index],arg1,arg2); Style: whitespace: printf_ln(x, y, z) > + } > + else { > + printf_ln(print_list[index],arg1,arg2,arg3); > + } Unfortunately, this is quite a bit more verbose and complex than the original code, and all the magic numbers (4, 2, 1, 0, 7, 4, 6) place a higher cognitive load on the reader, so this change probably is a net loss as far as clarity is concerned. Take a step back and consider again the GSoC miniproject: It talks about making the code table-driven. Certainly, you have moved the strings into a table, but all the complex logic is still in the code, and not in a table, hence it's not table-driven. To make this table-driven, you should try to figure out how most or all of this logic can be moved into a table. > + free(print_list); > + > + > +/* if (remote_is_branch && origin) > printf_ln(rebasing ? > _("Branch %s set up to track remote branch %s from %s by rebasing.") : > _("Branch %s set up to track remote branch %s from %s."), > @@ -100,6 +150,7 @@ void install_branch_config(int flag, const char *local, const char *origin, cons > else > die("BUG: impossible combination of %d and %p", > remote_is_branch, origin); > +*/ The code you wrote is meant to replace the old code, so your patch should actually remove the old code, not just comment it out. > } > } > > -- > 1.8.3.2 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] GSoC Change multiple if-else statements to be table-driven 2014-03-12 9:21 ` Eric Sunshine @ 2014-03-13 20:20 ` Yao Zhao 2014-03-14 2:16 ` Eric Sunshine 0 siblings, 1 reply; 13+ messages in thread From: Yao Zhao @ 2014-03-13 20:20 UTC (permalink / raw) To: sunshine; +Cc: git, Yao Zhao Signed-off-by: Yao Zhao <zhaox383@umn.edu> --- GSoC_MicroProject_#8 Hello Eric, Thanks for reviewing my code. I implemented table-driven method this time and correct the style problems indicated in review. Thank you, Yao branch.c | 72 ++++++++++++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 50 insertions(+), 22 deletions(-) diff --git a/branch.c b/branch.c index 723a36b..6451c99 100644 --- a/branch.c +++ b/branch.c @@ -49,10 +49,43 @@ static int should_setup_rebase(const char *origin) void install_branch_config(int flag, const char *local, const char *origin, const char *remote) { + const char *shortname = remote + 11; int remote_is_branch = starts_with(remote, "refs/heads/"); struct strbuf key = STRBUF_INIT; int rebasing = should_setup_rebase(origin); + int size=8, i; + + typedef struct PRINT_LIST { + const char *print_str; + const char *arg2; + //arg1 is always local, so I only add arg2 and arg3 in struct + const char *arg3; + int b_rebasing; + int b_remote_is_branch; + int b_origin; + } PRINT_LIST; + + PRINT_LIST print_list[] = { + {.print_str = _("Branch %s set up to track remote branch %s from %s by rebasing."), + .arg2 = shortname, .arg3 = origin, + .b_rebasing = 1, .b_remote_is_branch = 1, .b_origin = 1}, + {.print_str = _("Branch %s set up to track remote branch %s from %s."), + .arg2 = shortname, .arg3 = origin, + .b_rebasing = 0, .b_remote_is_branch = 1, .b_origin = 1}, + {.print_str = _("Branch %s set up to track local branch %s by rebasing."), + .arg2 = shortname, .b_rebasing = 1, .b_remote_is_branch = 1, .b_origin = 0}, + {.print_str = _("Branch %s set up to track local branch %s."), + .arg2 = shortname, .b_rebasing = 0, .b_remote_is_branch = 1, .b_origin = 0}, + {.print_str = _("Branch %s set up to track remote ref %s by rebasing."), + .arg2 = remote, .b_rebasing = 1, .b_remote_is_branch = 0, .b_origin = 1}, + {.print_str = _("Branch %s set up to track remote ref %s."), + .arg2 = remote, .b_rebasing = 0, .b_remote_is_branch = 0, .b_origin = 1}, + {.print_str = _("Branch %s set up to track local ref %s by rebasing."), + .arg2 = remote, .b_rebasing = 1, .b_remote_is_branch = 0, .b_origin = 0}, + {.print_str = _("Branch %s set up to track local ref %s."), + .arg2 = remote, .b_rebasing = 0, .b_remote_is_branch = 0, .b_origin = 0}, +}; I am confused here: I use struct initializer and I am not sure if it's ok because it is only supported by ANSI if (remote_is_branch && !strcmp(local, shortname) @@ -75,31 +108,26 @@ void install_branch_config(int flag, const char *local, const char *origin, cons git_config_set(key.buf, "true"); } strbuf_release(&key); - if (flag & BRANCH_CONFIG_VERBOSE) { - if (remote_is_branch && origin) - printf_ln(rebasing ? - _("Branch %s set up to track remote branch %s from %s by rebasing.") : - _("Branch %s set up to track remote branch %s from %s."), - local, shortname, origin); - else if (remote_is_branch && !origin) - printf_ln(rebasing ? - _("Branch %s set up to track local branch %s by rebasing.") : - _("Branch %s set up to track local branch %s."), - local, shortname); - else if (!remote_is_branch && origin) - printf_ln(rebasing ? - _("Branch %s set up to track remote ref %s by rebasing.") : - _("Branch %s set up to track remote ref %s."), - local, remote); - else if (!remote_is_branch && !origin) - printf_ln(rebasing ? - _("Branch %s set up to track local ref %s by rebasing.") : - _("Branch %s set up to track local ref %s."), - local, remote); - else + for (i=0;i<size;i++) + { + if (print_list[i].b_rebasing == (rebasing? 1 : 0) && + print_list[i].b_remote_is_branch == (remote_is_branch? 1 : 0) && + print_list[i].b_origin == (origin? 1 : 0)) + { + if (print_list[i].arg3 == NULL) + printf_ln (print_list[i].print_str, local, print_list[i].arg2); + else + printf_ln (print_list[i].print_str, local, + print_list[i].arg2, print_list[i].arg3); + + break; + } + } + if (i == size) die("BUG: impossible combination of %d and %p", remote_is_branch, origin); + } } -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] GSoC Change multiple if-else statements to be table-driven 2014-03-13 20:20 ` [PATCH] GSoC Change multiple if-else statements to be table-driven Yao Zhao @ 2014-03-14 2:16 ` Eric Sunshine 2014-03-14 16:54 ` Junio C Hamano 2014-03-16 6:08 ` [PATCH/GSoC_v3] branch.c: turn nested if-else logic to table-driven Yao Zhao 0 siblings, 2 replies; 13+ messages in thread From: Eric Sunshine @ 2014-03-14 2:16 UTC (permalink / raw) To: Yao Zhao; +Cc: Git List Thanks for the resubmission. Comments below. On Thu, Mar 13, 2014 at 4:20 PM, Yao Zhao <zhaox383@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 will be [PATCH v3]. The -v option for "git format-email" can help. When your patch is applied via "git am", text inside [...] gets stripped automatically. The "GSoC" tells email readers what this submission is about, but isn't relevant to the actual commit message. It should be placed inside [...]. For instance: [PATCH/GSoC v2]. > Signed-off-by: Yao Zhao <zhaox383@umn.edu> > --- > GSoC_MicroProject_#8 > > Hello Eric, > > Thanks for reviewing my code. I implemented table-driven method this time and correct the style > problems indicated in review. Explaining what you changed since the last version is indeed good etiquette. Thanks. For bonus points, also provide a link to the previous version, like this [1]. [1]: http://thread.gmane.org/gmane.comp.version-control.git/243919 > branch.c | 72 ++++++++++++++++++++++++++++++++++++++++++++-------------------- > 1 file changed, 50 insertions(+), 22 deletions(-) > > diff --git a/branch.c b/branch.c > index 723a36b..6451c99 100644 > --- a/branch.c > +++ b/branch.c > @@ -49,10 +49,43 @@ static int should_setup_rebase(const char *origin) > > void install_branch_config(int flag, const char *local, const char *origin, const char *remote) > { > + Unnecessary insertion of blank line. > const char *shortname = remote + 11; > int remote_is_branch = starts_with(remote, "refs/heads/"); > struct strbuf key = STRBUF_INIT; > int rebasing = should_setup_rebase(origin); > + int size=8, i; Style: whitespace: size = 8 You can use ARRAY_SIZE(print_list) to avoid hardcoding 8 (and then you don't need the variable 'size'). > + typedef struct PRINT_LIST { > + const char *print_str; > + const char *arg2; > + //arg1 is always local, so I only add arg2 and arg3 in struct This commentary should be placed below the "---" under your sign-off (or dropped altogether since it's pretty obvious). Also, in this project avoid //-style comments. > + const char *arg3; > + int b_rebasing; > + int b_remote_is_branch; Strange indentation. Use tabs for indentation, and set your editor so tabs have width 8. > + int b_origin; > + } PRINT_LIST; Read below for some commentary about b_rebasing, b_remote_is_branch, b_origin. > + PRINT_LIST print_list[] = { > + {.print_str = _("Branch %s set up to track remote branch %s from %s by rebasing."), > + .arg2 = shortname, .arg3 = origin, > + .b_rebasing = 1, .b_remote_is_branch = 1, .b_origin = 1}, > + {.print_str = _("Branch %s set up to track remote branch %s from %s."), > + .arg2 = shortname, .arg3 = origin, > + .b_rebasing = 0, .b_remote_is_branch = 1, .b_origin = 1}, > + {.print_str = _("Branch %s set up to track local branch %s by rebasing."), > + .arg2 = shortname, .b_rebasing = 1, .b_remote_is_branch = 1, .b_origin = 0}, > + {.print_str = _("Branch %s set up to track local branch %s."), > + .arg2 = shortname, .b_rebasing = 0, .b_remote_is_branch = 1, .b_origin = 0}, > + {.print_str = _("Branch %s set up to track remote ref %s by rebasing."), > + .arg2 = remote, .b_rebasing = 1, .b_remote_is_branch = 0, .b_origin = 1}, > + {.print_str = _("Branch %s set up to track remote ref %s."), > + .arg2 = remote, .b_rebasing = 0, .b_remote_is_branch = 0, .b_origin = 1}, > + {.print_str = _("Branch %s set up to track local ref %s by rebasing."), > + .arg2 = remote, .b_rebasing = 1, .b_remote_is_branch = 0, .b_origin = 0}, > + {.print_str = _("Branch %s set up to track local ref %s."), > + .arg2 = remote, .b_rebasing = 0, .b_remote_is_branch = 0, .b_origin = 0}, > +}; > I am confused here: I use struct initializer and I am not sure if it's ok > because it is only supported by ANSI This commentary should be placed below "---" after your sign-off. Indeed, you want to avoid named field initializers in this project and instead use positional initializers. Translatable strings in an initializer should be wrapped with N_() instead of _(). You will still need to use _() later on when you reference the string from the table. See section 4.7 [2] of the GNU gettext manual for details. [2]: http://www.gnu.org/software/gettext/manual/gettext.html#Special-cases > if (remote_is_branch > && !strcmp(local, shortname) > @@ -75,31 +108,26 @@ void install_branch_config(int flag, const char *local, const char *origin, cons > git_config_set(key.buf, "true"); > } > strbuf_release(&key); > - > if (flag & BRANCH_CONFIG_VERBOSE) { > - if (remote_is_branch && origin) > - printf_ln(rebasing ? > - _("Branch %s set up to track remote branch %s from %s by rebasing.") : > - _("Branch %s set up to track remote branch %s from %s."), > - local, shortname, origin); > - else if (remote_is_branch && !origin) > - printf_ln(rebasing ? > - _("Branch %s set up to track local branch %s by rebasing.") : > - _("Branch %s set up to track local branch %s."), > - local, shortname); > - else if (!remote_is_branch && origin) > - printf_ln(rebasing ? > - _("Branch %s set up to track remote ref %s by rebasing.") : > - _("Branch %s set up to track remote ref %s."), > - local, remote); > - else if (!remote_is_branch && !origin) > - printf_ln(rebasing ? > - _("Branch %s set up to track local ref %s by rebasing.") : > - _("Branch %s set up to track local ref %s."), > - local, remote); > - else > + for (i=0;i<size;i++) Style: whitespace: for (i = 0; i < size; i++) > + { > + if (print_list[i].b_rebasing == (rebasing? 1 : 0) && > + print_list[i].b_remote_is_branch == (remote_is_branch? 1 : 0) && > + print_list[i].b_origin == (origin? 1 : 0)) Style: whitespace: (x ? 1 : 0) Assigning &print_list[i] to a variable would allow you to reduce the noise of this expression a bit. On this project it is more idiomatic to say !!rebasing, !!remote_is_branch, !!origin to constrain these values to 0 or 1. An alternate approach might be to use a multi-dimensional array, where the boolean values of rebasing, remote_is_branch, and origin are keys into the array. This would allow you to pick out the correct PRINT_LIST entry directly (no looping), thus eliminating the need for those b_rebasing, b_remote_is_branch, and b_origin members. > + { > + if (print_list[i].arg3 == NULL) > + printf_ln (print_list[i].print_str, local, print_list[i].arg2); Style: whitespace: printf_ln(...) Reminder: wrap _() around print_list[i].print_str > + else > + printf_ln (print_list[i].print_str, local, > + print_list[i].arg2, print_list[i].arg3); This logic can be simplified. Hint: It is not an error to pass more arguments than there are %s's in the format string. > + > + break; > + } > + } > + if (i == size) > die("BUG: impossible combination of %d and %p", > remote_is_branch, origin); > + Unnecessary insertion of blank line. > } > } > > -- > 1.8.3.2 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] GSoC Change multiple if-else statements to be table-driven 2014-03-14 2:16 ` Eric Sunshine @ 2014-03-14 16:54 ` Junio C Hamano 2014-03-17 6:29 ` Eric Sunshine 2014-03-16 6:08 ` [PATCH/GSoC_v3] branch.c: turn nested if-else logic to table-driven Yao Zhao 1 sibling, 1 reply; 13+ messages in thread From: Junio C Hamano @ 2014-03-14 16:54 UTC (permalink / raw) To: Eric Sunshine; +Cc: Yao Zhao, Git List Eric Sunshine <sunshine@sunshineco.com> writes: > Thanks for the resubmission. Comments below. Thanks, Eric, for helping so many micro exercises. > On Thu, Mar 13, 2014 at 4:20 PM, Yao Zhao <zhaox383@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 will be [PATCH v3]. The -v option > for "git format-email" can help. Yao, I think Eric meant "git format-patch". > When your patch is applied via "git am", text inside [...] gets > stripped automatically. The "GSoC" tells email readers what this > submission is about, but isn't relevant to the actual commit message. > It should be placed inside [...]. For instance: [PATCH/GSoC v2]. So in short, Subject: [PATCH/GSoC v2] branch.c: turn nested if-else logic to table-driven or something. >> + typedef struct PRINT_LIST { > ... >> + int b_origin; >> + } PRINT_LIST; We do not do ALL_CAPS names and tend not to introduce one-off typedefs for struct. Instead we would just use "struct print_list" throughout (if we were to indeed use such a new struct, that is). >> + PRINT_LIST print_list[] = { >> + {.print_str = _("Branch %s set up to track remote branch %s from %s by rebasing."), >> + .arg2 = shortname, .arg3 = origin, >> + .b_rebasing = 1, .b_remote_is_branch = 1, .b_origin = 1}, >> I am confused here: I use struct initializer and I am not sure if it's ok >> because it is only supported by ANSI > ... > Indeed, you want to avoid named field initializers in this project and > instead use positional initializers. Correct. > Translatable strings in an initializer should be wrapped with N_() > instead of _(). You will still need to use _() later on when you > reference the string from the table. See section 4.7 [2] of the GNU > gettext manual for details. Correct. > An alternate approach might be to use a multi-dimensional array, > where the boolean values of rebasing, remote_is_branch, and origin > are keys into the array. This would allow you to pick out the > correct PRINT_LIST entry directly (no looping), thus eliminating > the need for those b_rebasing, b_remote_is_branch, and b_origin > members. Correct. After seeing so many "table driven" submissions, I however tend to agree with your earlier comment on another thread on this same micro, where you said an nested if-else cascade that was rewritten in a clearer way (sorry, I do not remember whose submission it was offhand) may be the best answer to the "Would it make sense to make the code table-driven?" question, even though I tentatively queued d7ea7894 (install_branch_config(): simplify verbose messages logic, 2014-03-13) from Paweł on 'pu'. Thanks for a review. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] GSoC Change multiple if-else statements to be table-driven 2014-03-14 16:54 ` Junio C Hamano @ 2014-03-17 6:29 ` Eric Sunshine 2014-03-17 7:31 ` Junio C Hamano 0 siblings, 1 reply; 13+ messages in thread From: Eric Sunshine @ 2014-03-17 6:29 UTC (permalink / raw) To: Junio C Hamano; +Cc: Yao Zhao, Git List, Adam, Michael Haggerty On Fri, Mar 14, 2014 at 12:54 PM, Junio C Hamano <gitster@pobox.com> wrote: > Eric Sunshine <sunshine@sunshineco.com> writes: > >> On Thu, Mar 13, 2014 at 4:20 PM, Yao Zhao <zhaox383@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 will be [PATCH v3]. The -v option >> for "git format-email" can help. > > Yao, I think Eric meant "git format-patch". Correct. Sorry for the confusion. >> An alternate approach might be to use a multi-dimensional array, >> where the boolean values of rebasing, remote_is_branch, and origin >> are keys into the array. This would allow you to pick out the >> correct PRINT_LIST entry directly (no looping), thus eliminating >> the need for those b_rebasing, b_remote_is_branch, and b_origin >> members. > > Correct. > > After seeing so many "table driven" submissions, I however tend to > agree with your earlier comment on another thread on this same > micro, where you said an nested if-else cascade that was rewritten > in a clearer way (sorry, I do not remember whose submission it was It was Adam NoLastName [1]. [1]: http://thread.gmane.org/gmane.comp.version-control.git/243704 > offhand) may be the best answer to the "Would it make sense to make > the code table-driven?" question, even though I tentatively queued > d7ea7894 (install_branch_config(): simplify verbose messages logic, > 2014-03-13) from Paweł on 'pu'. Perhaps it is time to mark this microproject as "taken" on the GSoC page [2], along a fews others for which we have received multiple submissions. [2]: https://github.com/git/git.github.io/blob/master/SoC-2014-Microprojects.md ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] GSoC Change multiple if-else statements to be table-driven 2014-03-17 6:29 ` Eric Sunshine @ 2014-03-17 7:31 ` Junio C Hamano 2014-03-17 14:11 ` Michael Haggerty 0 siblings, 1 reply; 13+ messages in thread From: Junio C Hamano @ 2014-03-17 7:31 UTC (permalink / raw) To: Eric Sunshine; +Cc: Yao Zhao, Git List, Adam, Michael Haggerty Eric Sunshine <sunshine@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 multiple > submissions. > > [2]: https://github.com/git/git.github.io/blob/master/SoC-2014-Microprojects.md I actually have been of multiple minds on this. * After seeing that many candidates tried to solve the same micro, apparently without checking answers by other people, and seeing how they responded to the reviews given to them, I found that we had as equally good interactions with them to judge their skills, both techincal and social, as we would have had if each of them solved different micros. * Many reviewers may have gotten tired of seeing many novice attempts on the same micro over and over again, giving gentle suggestions for improvements. Because the _sole_ purpose of these micros were to help candidates get their toes wet in the process, duplicated efforts on the candidates' side are not wasted---they each hopefully learned how to interact with this community. But it is true that, if we were wishing to also get some trivial clean-ups in the codebase as a side effect of these micros, we have wasted reviewer bandwidth by not having enough micros, and reviewers may have had some feeling that their efforts did not fully contribute to improving our codebase, which may have been discouraging. Big thanks go to all reviewers who participated despite this. If we had more micros, the apparent wastage of the reviewer efforts might have been avoided. * Many candidates did not even bother to check if others are working on the same micro, however, which would be a bad sign by itself. Concentrating on one's own topic, without paying any attention to what others are working on the same software, is never a good discipline. Some may argue that it would be unfair to blame the candidates on this---they may have picked up micros that haven't been solved if we had more---but after seeing that many candidates' submissions apparently did not take into account the reviews given to others' submissions on the same micro and/or had many exactly the same issues like log message styles as submissions on other micros that have already been reviewed, I personally do not think they are blameless. So in short, yes it would have been nicer if we had more micros than candidates, but I do not think it was detrimental for the purpose of these micro exercises that multiple candidates ended up attempting the same micro. Again, Big Thanks to Michael for the excellent "micro" idea, and all reviewers who participated. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] GSoC Change multiple if-else statements to be table-driven 2014-03-17 7:31 ` Junio C Hamano @ 2014-03-17 14:11 ` Michael Haggerty 2014-03-17 19:12 ` Junio C Hamano 2014-03-17 19:27 ` Eric Sunshine 0 siblings, 2 replies; 13+ messages in thread From: Michael Haggerty @ 2014-03-17 14:11 UTC (permalink / raw) To: Junio C Hamano; +Cc: Eric Sunshine, Git List On 03/17/2014 08:31 AM, Junio C Hamano wrote: > Eric Sunshine <sunshine@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 multiple >> submissions. I just marked #8 as taken, as it's been beaten to death. I haven't been keeping careful track of which other microprojects have been overused. If you have suggestions for the chopping block, let me know. >> [2]: https://github.com/git/git.github.io/blob/master/SoC-2014-Microprojects.md > > I actually have been of multiple minds on this. > > * After seeing that many candidates tried to solve the same micro, > apparently without checking answers by other people, and seeing > how they responded to the reviews given to them, I found that we > had as equally good interactions with them to judge their skills, > both techincal and social, as we would have had if each of them > solved different micros. > > * Many reviewers may have gotten tired of seeing many novice > attempts on the same micro over and over again, giving gentle > suggestions for improvements. Because the _sole_ purpose of these > micros were to help candidates get their toes wet in the process, > duplicated efforts on the candidates' side are not wasted---they > each hopefully learned how to interact with this community. > > But it is true that, if we were wishing to also get some trivial > clean-ups in the codebase as a side effect of these micros, we > have wasted reviewer bandwidth by not having enough micros, and > reviewers may have had some feeling that their efforts did not > fully contribute to improving our codebase, which may have been > discouraging. > > Big thanks go to all reviewers who participated despite this. If > we had more micros, the apparent wastage of the reviewer efforts > might have been avoided. > > * Many candidates did not even bother to check if others are > working on the same micro, however, which would be a bad sign by > itself. Concentrating on one's own topic, without paying any > attention to what others are working on the same software, is > never a good discipline. > > Some may argue that it would be unfair to blame the candidates on > this---they may have picked up micros that haven't been solved if > we had more---but after seeing that many candidates' submissions > apparently did not take into account the reviews given to others' > submissions on the same micro and/or had many exactly the same > issues like log message styles as submissions on other micros > that have already been reviewed, I personally do not think they > are blameless. > > So in short, yes it would have been nicer if we had more micros than > candidates, but I do not think it was detrimental for the purpose of > these micro exercises that multiple candidates ended up attempting > the same micro. I wish I had had time to invent more microprojects. I think we were lucky: since students didn't seem to learn from each other's attempts, the fact that many people tried the same microprojects wasn't much of a problem. But if a student had arrived with a "perfect" solution to a well-trodden microproject on his/her first attempt, I would hate to have to be suspicious that the student plagiarized from somebody else's answer plus all of the review comments about the earlier answer, especially since a perfect solution would itself reduce the amount of interaction between the cheating student and the mailing list compared to a student who required several iterations. I also hope that students didn't feel unwelcomed during the times when the list of untaken microprojects was nearly empty. If we really wanted to make this system cheat-proof, there would have to be not only enough microprojects to go around, but also some mechanism to ensure that student B didn't start work on a microproject that student A was just about to submit a solution to. Maybe students would have to claim a microproject when they started work on them, or request a microproject to work on as opposed to picking one from a list on a web page. But realistically, I think that this hypothetical improved system would be more work than we would have been able to invest. Maybe in the future our microprojects should have two parts: a. Fix ... b. Invent a microproject for the next student. (Just kidding.) > Again, Big Thanks to Michael for the excellent "micro" idea, and all > reviewers who participated. I'd really like to thank the reviewers, who put in enormous effort reviewing submissions promptly, and showed superhuman patience when dealing with the same issues over an over again. Without such great reviewers the whole idea would have just resulted in frustration for the students; with them, I think that even the students who don't get a GSoC spot will have learned something valuable from their participation. Michael -- Michael Haggerty mhagger@alum.mit.edu http://softwareswirl.blogspot.com/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] GSoC Change multiple if-else statements to be table-driven 2014-03-17 14:11 ` Michael Haggerty @ 2014-03-17 19:12 ` Junio C Hamano 2014-03-17 19:27 ` Eric Sunshine 1 sibling, 0 replies; 13+ messages in thread From: Junio C Hamano @ 2014-03-17 19:12 UTC (permalink / raw) To: Michael Haggerty; +Cc: Eric Sunshine, Git List Michael Haggerty <mhagger@alum.mit.edu> writes: > On 03/17/2014 08:31 AM, Junio C Hamano wrote: > >> So in short, yes it would have been nicer if we had more micros than >> candidates, but I do not think it was detrimental for the purpose of >> these micro exercises that multiple candidates ended up attempting >> the same micro. > > I wish I had had time to invent more microprojects. I think we were > lucky: since students didn't seem to learn from each other's attempts, > the fact that many people tried the same microprojects wasn't much of a > problem. But if a student had arrived with a "perfect" solution to a > well-trodden microproject on his/her first attempt, I would hate to have > to be suspicious that the student plagiarized from somebody else's > answer plus all of the review comments about the earlier answer, > especially since a perfect solution would itself reduce the amount of > interaction between the cheating student and the mailing list compared > to a student who required several iterations. It may probably be not a huge problem. If anything, a cheater would have also learned how the mailing interaction goes when trying to plagiarise. And not really having interacted us, a cheater would have left us no impression on his or her communication skills, so it would probably have been detrimental for his or her own chance to be chosen as a student, compared to the ones who started from their own solution and polished it by responding to reviews. > I also hope that students > didn't feel unwelcomed during the times when the list of untaken > microprojects was nearly empty. Yeah, that is why I said I was of multiple minds. I wasn't enthused by seeing the ones that somebody is attempting marked as "taken" in the first place. Solving one that others attempted in his or her own way and interacting with reviewers seemed to have just as much information on the candidate, at least to me. > If we really wanted to make this system cheat-proof, there would have to > be not only enough microprojects to go around, but also some mechanism > to ensure that student B didn't start work on a microproject that > student A was just about to submit a solution to. Nah, I do not think there is any need to go there. It is over for this year anyway, but I do not think such a provision is necessary for future years, if Git project will participate in future GSoCs; see above. > Maybe in the future our microprojects should have two parts: > > a. Fix ... > b. Invent a microproject for the next student. > > (Just kidding.) ;-) ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] GSoC Change multiple if-else statements to be table-driven 2014-03-17 14:11 ` Michael Haggerty 2014-03-17 19:12 ` Junio C Hamano @ 2014-03-17 19:27 ` Eric Sunshine 2014-03-17 20:39 ` Quint Guvernator 1 sibling, 1 reply; 13+ messages in thread From: Eric Sunshine @ 2014-03-17 19:27 UTC (permalink / raw) To: Michael Haggerty; +Cc: Junio C Hamano, Git List On Mon, Mar 17, 2014 at 10:11 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote: > On 03/17/2014 08:31 AM, Junio C Hamano wrote: >> Eric Sunshine <sunshine@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 multiple >>> submissions. > > I just marked #8 as taken, as it's been beaten to death. > > I haven't been keeping careful track of which other microprojects have > been overused. If you have suggestions for the chopping block, let me know. A quick (perhaps inaccurate) search of the mailing list shows that, of the remaining "untaken" items, #10, 11, 12, 15, 16, and 18 have had just one submission, and #13 had two, so we're okay. (When I wrote the above, I was probably thinking of some of the earlier items on the list which we saw submitted repeatedly.) ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] GSoC Change multiple if-else statements to be table-driven 2014-03-17 19:27 ` Eric Sunshine @ 2014-03-17 20:39 ` Quint Guvernator 0 siblings, 0 replies; 13+ messages in thread From: Quint Guvernator @ 2014-03-17 20:39 UTC (permalink / raw) To: Eric Sunshine; +Cc: Michael Haggerty, Junio C Hamano, Git List 2014-03-17 15:27 GMT-04:00 Eric Sunshine <sunshine@sunshineco.com>: > A quick (perhaps inaccurate) search of the mailing list shows that, of > the remaining "untaken" items, #10, 11, 12, 15, 16, and 18 have had > just one submission, and #13 had two, so we're okay. I am still working on #14: "Change fetch-pack.c:filter_refs() to use starts_with() instead of memcmp(). Try to find other sites that could be rewritten similarly." Another version of the patch should be on this list within the hour. Quint ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH/GSoC_v3] branch.c: turn nested if-else logic to table-driven 2014-03-14 2:16 ` Eric Sunshine 2014-03-14 16:54 ` Junio C Hamano @ 2014-03-16 6:08 ` Yao Zhao 2014-03-17 7:54 ` Eric Sunshine 1 sibling, 1 reply; 13+ messages in thread From: Yao Zhao @ 2014-03-16 6:08 UTC (permalink / raw) To: sunshine; +Cc: git, Yao Zhao Signed-off-by: Yao Zhao <zhaox383@umn.edu> --- branch.c | 53 +++++++++++++++++++++++++++++------------------------ 1 file changed, 29 insertions(+), 24 deletions(-) Hello Eric, Thank you and Junio for reviewing my code. It is really helpful to improve my code quality. This is version 3 of patch. Previous address : http://thread.gmane.org/gmane.comp.version-control.git/243919. I do not use positional initializer because it is not allowed to use variable in it. I don't know if it's ok to use this redundant way to initialize "list". I cannot find -v flag in documentation you indicated in last email so I use set-prefix to add it into prefix. Now I am working on writing proposal for git project. I am really interested in last one, about improve git_config. I know it's important to get known about git_config first and have read documentation about it. But I am really confused about how to understand code of git_config. When user type in git config in terminal, what is the execute order of functions? How git config influence other git command? Does program read config file every time when they execuate config-related command? Thank you, Yao diff --git a/branch.c b/branch.c index 723a36b..1df30c7 100644 --- a/branch.c +++ b/branch.c @@ -53,7 +53,33 @@ void install_branch_config(int flag, const char *local, const char *origin, cons int remote_is_branch = starts_with(remote, "refs/heads/"); struct strbuf key = STRBUF_INIT; int rebasing = should_setup_rebase(origin); - + struct print_list { + const char *print_str; + const char *arg2; + const char *arg3; + } ; + struct print_list target; + + struct print_list list[2][2][2]; + list[0][0][0].print_str = N_("Branch %s set up to track local ref %s."); + list[0][0][0].arg2 = remote; + list[0][0][1].print_str = N_("Branch %s set up to track local ref %s by rebasing."); + list[0][0][1].arg2 = remote; + list[0][1][0].print_str = N_("Branch %s set up to track remote ref %s."); + list[0][1][0].arg2 = remote; + list[0][1][1].print_str = N_("Branch %s set up to track remote ref %s by rebasing."); + list[0][1][1].arg2 = remote; + list[1][0][0].print_str = N_("Branch %s set up to track local branch %s."); + list[1][0][0].arg2 =shortname; + list[1][0][1].print_str = N_("Branch %s set up to track local branch %s by rebasing."); + list[1][0][1].arg2 = shortname; + list[1][1][0].print_str = N_("Branch %s set up to track remote branch %s from %s."); + list[1][1][0].arg2 = shortname; + list[1][1][0].arg3 = origin; + list[1][1][1].print_str = N_("Branch %s set up to track remote branch %s from %s by rebasing."); + list[1][1][1].arg2 = shortname; + list[1][1][1].arg3 = origin; + if (remote_is_branch && !strcmp(local, shortname) && !origin) { @@ -77,29 +103,8 @@ void install_branch_config(int flag, const char *local, const char *origin, cons strbuf_release(&key); if (flag & BRANCH_CONFIG_VERBOSE) { - if (remote_is_branch && origin) - printf_ln(rebasing ? - _("Branch %s set up to track remote branch %s from %s by rebasing.") : - _("Branch %s set up to track remote branch %s from %s."), - local, shortname, origin); - else if (remote_is_branch && !origin) - printf_ln(rebasing ? - _("Branch %s set up to track local branch %s by rebasing.") : - _("Branch %s set up to track local branch %s."), - local, shortname); - else if (!remote_is_branch && origin) - printf_ln(rebasing ? - _("Branch %s set up to track remote ref %s by rebasing.") : - _("Branch %s set up to track remote ref %s."), - local, remote); - else if (!remote_is_branch && !origin) - printf_ln(rebasing ? - _("Branch %s set up to track local ref %s by rebasing.") : - _("Branch %s set up to track local ref %s."), - local, remote); - else - die("BUG: impossible combination of %d and %p", - remote_is_branch, origin); + target = list[!!remote_is_branch][!!origin][!!rebasing]; + printf_ln (_(target.print_str), local, target.arg2, target.arg3); } } -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH/GSoC_v3] branch.c: turn nested if-else logic to table-driven 2014-03-16 6:08 ` [PATCH/GSoC_v3] branch.c: turn nested if-else logic to table-driven Yao Zhao @ 2014-03-17 7:54 ` Eric Sunshine 0 siblings, 0 replies; 13+ messages in thread From: Eric Sunshine @ 2014-03-17 7:54 UTC (permalink / raw) To: Yao Zhao; +Cc: Git List On Sun, Mar 16, 2014 at 2:08 AM, Yao Zhao <zhaox383@umn.edu> wrote: > Signed-off-by: Yao Zhao <zhaox383@umn.edu> > --- > branch.c | 53 +++++++++++++++++++++++++++++------------------------ > 1 file changed, 29 insertions(+), 24 deletions(-) > Hello Eric, > > Thank you and Junio for reviewing my code. It is really helpful to improve my code quality. Wrap commit message (and preferably commentary) to 65-70 characters. > This is version 3 of patch. Previous address : http://thread.gmane.org/gmane.comp.version-control.git/243919. I do not use positional initializer because it is not allowed to use variable in it. I don't know if it's ok to use this redundant way to initialize "list". Thanks for the resubmission. A few comments below, but I don't think you need to resubmit. What is important is that you have had a taste of the review process on this project, and the GSoC mentors have had a chance to observe your abilities and reviewer interaction. A better use of your time now would be to make your GSoC proposal and converse with the mentors. > I cannot find -v flag in documentation you indicated in last email so I use set-prefix to add it into prefix. Sorry for the confusion. As Junio pointed out [1], I meant the -v option of "git format-patch", not "git format-email" (which doesn't exist). [1]: http://thread.gmane.org/gmane.comp.version-control.git/243919/focus=244095 > Now I am working on writing proposal for git project. I am really interested in last one, about improve git_config. I know it's important to get known about git_config first and have read documentation about it. But I am really confused about how to understand code of git_config. When user type in git config in terminal, what is the execute order of functions? How git config influence other git command? Does program read config file every time when they execuate config-related command? You will get a better response if you ask these questions in a new email thread rather than re-using this one. Choose a subject for your email which indicates clearly that you have questions about that particular GSoC project, address it to the git mailing list, and cc: the mentors of that project. > Thank you, > > Yao > > diff --git a/branch.c b/branch.c > index 723a36b..1df30c7 100644 > --- a/branch.c > +++ b/branch.c > @@ -53,7 +53,33 @@ void install_branch_config(int flag, const char *local, const char *origin, cons > int remote_is_branch = starts_with(remote, "refs/heads/"); > struct strbuf key = STRBUF_INIT; > int rebasing = should_setup_rebase(origin); > - > + struct print_list { > + const char *print_str; > + const char *arg2; > + const char *arg3; > + } ; > + struct print_list target; This should be 'const struct print_list *target'. There is no need to copy the entire structure (below) just to access its members. > + struct print_list list[2][2][2]; > + list[0][0][0].print_str = N_("Branch %s set up to track local ref %s."); > + list[0][0][0].arg2 = remote; > + list[0][0][1].print_str = N_("Branch %s set up to track local ref %s by rebasing."); > + list[0][0][1].arg2 = remote; > + list[0][1][0].print_str = N_("Branch %s set up to track remote ref %s."); > + list[0][1][0].arg2 = remote; > + list[0][1][1].print_str = N_("Branch %s set up to track remote ref %s by rebasing."); > + list[0][1][1].arg2 = remote; > + list[1][0][0].print_str = N_("Branch %s set up to track local branch %s."); > + list[1][0][0].arg2 =shortname; > + list[1][0][1].print_str = N_("Branch %s set up to track local branch %s by rebasing."); > + list[1][0][1].arg2 = shortname; > + list[1][1][0].print_str = N_("Branch %s set up to track remote branch %s from %s."); > + list[1][1][0].arg2 = shortname; > + list[1][1][0].arg3 = origin; > + list[1][1][1].print_str = N_("Branch %s set up to track remote branch %s from %s by rebasing."); > + list[1][1][1].arg2 = shortname; > + list[1][1][1].arg3 = origin; Since this is not in an initializer table, you would normally use _() rather than N_() (and you don't have to use _() in the printf_ln() invocation). It disturbs me to see that .arg3 does not get initialized in most of these cases, yet it is accessed below by printf_ln(). Code analysis tools are likely to complain about accessing an uninitialized value. One way you could put this into the initializer would be to use a constant expression to indicate which variable to pass to printf_ln(). Something like this: struct print_list { const char *print_str; int use_shortname; int use_origin; } print_list[][2][2] = { N_("Branch %s ..."), 0, 0, ... N_("Branch %s ..."), 1, 0, ... } printf_ln(_(target->print_str), local, target->use_shortname ? shortname : remote, target->use_origin ? origin : NULL); To make it clearer, use named constants rather than 0 and 1. NOTE: I am *not* suggesting that you resubmit with this change. It's getting ugly and bordering on being too complex and difficult to read. As noted above, your time is probably better spent working on your GSoC proposal. > if (remote_is_branch > && !strcmp(local, shortname) > && !origin) { > @@ -77,29 +103,8 @@ void install_branch_config(int flag, const char *local, const char *origin, cons > strbuf_release(&key); > > if (flag & BRANCH_CONFIG_VERBOSE) { > - if (remote_is_branch && origin) > - printf_ln(rebasing ? > - _("Branch %s set up to track remote branch %s from %s by rebasing.") : > - _("Branch %s set up to track remote branch %s from %s."), > - local, shortname, origin); > - else if (remote_is_branch && !origin) > - printf_ln(rebasing ? > - _("Branch %s set up to track local branch %s by rebasing.") : > - _("Branch %s set up to track local branch %s."), > - local, shortname); > - else if (!remote_is_branch && origin) > - printf_ln(rebasing ? > - _("Branch %s set up to track remote ref %s by rebasing.") : > - _("Branch %s set up to track remote ref %s."), > - local, remote); > - else if (!remote_is_branch && !origin) > - printf_ln(rebasing ? > - _("Branch %s set up to track local ref %s by rebasing.") : > - _("Branch %s set up to track local ref %s."), > - local, remote); > - else > - die("BUG: impossible combination of %d and %p", > - remote_is_branch, origin); > + target = list[!!remote_is_branch][!!origin][!!rebasing]; With the suggestion above of making 'target' a pointer to the struct, this would become: target = &list[...][...][...]; > + printf_ln (_(target.print_str), local, target.arg2, target.arg3); Style: whitespace: printf_ln(...) And this would become: printf_ln(_(target->print_str), local, target->arg2, target->arg3); > } > } > > -- > 1.8.3.2 ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2014-03-17 20:40 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-03-12 3:47 [PATCH] SoC 2014 MicroProject No.8:change multiple if-else statement to table-driven approach Yao Zhao 2014-03-12 9:21 ` Eric Sunshine 2014-03-13 20:20 ` [PATCH] GSoC Change multiple if-else statements to be table-driven Yao Zhao 2014-03-14 2:16 ` Eric Sunshine 2014-03-14 16:54 ` Junio C Hamano 2014-03-17 6:29 ` Eric Sunshine 2014-03-17 7:31 ` Junio C Hamano 2014-03-17 14:11 ` Michael Haggerty 2014-03-17 19:12 ` Junio C Hamano 2014-03-17 19:27 ` Eric Sunshine 2014-03-17 20:39 ` Quint Guvernator 2014-03-16 6:08 ` [PATCH/GSoC_v3] branch.c: turn nested if-else logic to table-driven Yao Zhao 2014-03-17 7:54 ` Eric Sunshine
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).