Jared Hance wrote:
> [PATCH v3 2/3] Add function per_branch_config.
>
> Adds a configuration function to be filled up more in the next patch.
Micronit: subject should be of the form
subsystem: do something really great
with no full stop. So the series might be, roughly:
- merge: introduce setup_merge_commit helper function
- merge: introduce per-branch configuration helper function
- merge: add support for merging from upstream by default
Commit message bodies tend to be in the imperative mood, just like the
patches themselves.
Anyway, I like patch 1. One tiny nit on this one:
> +++ b/builtin/merge.c
> @@ -498,11 +498,15 @@ cleanup:
> strbuf_release(&bname);
> }
>
> -static int git_merge_config(const char *k, const char *v, void *cb)
> +static int per_branch_config(const char *k, const char *v, void *cb)
> {
> - if (branch && !prefixcmp(k, "branch.") &&
> - !prefixcmp(k + 7, branch) &&
> - !strcmp(k + 7 + strlen(branch), ".mergeoptions")) {
> + const char *variable;
> + if (!branch || prefixcmp(k, "branch.")
> + || prefixcmp(k + 7, branch))
> + return 1; /* not what I handle */
Alignment: isn't the "||" meant to be one space over?
Based on a quick grep[1], it seems the prevailing style is rather
if (!branch || prefixcmp(k, "branch.") ||
prefixcmp(k + 7, branch))
return 1;
with the || at the end of the line, though I'm not sure I care
much. :)
With or without an extra space before the ||,
Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
Thanks for a nice cleanup.
Jonathan
[1]
addup () {
sum=0
while read term
do
: $((sum = sum + term))
done
echo $sum
}
$ git grep --cached -c -e '||$' | cut -d: -f2 | addup
730
$ git grep --cached -c -e ' ||' | cut -d: -f2 | addup
98
$ git grep --cached -c -e ' ||' | cut -d: -f2 | addup
79
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html