Heikki Orsila [off-list ref] writes:
On Fri, Aug 29, 2008 at 05:56:58PM -0700, Junio C Hamano wrote:
...
quoted
I do not think a single patch with the above hunk (which I think is a
mistake) and other bits that are obviously good (e.g. the first hunk to
builtin-apply.c we see below) is reviewable, but I cannot think of a
better alterantive. Sigh...
...
quoted
I did not look at the rest.
Is the concept OK for this change? I can submit another patch.
By "concept" if you mean the goal to stop implying to the end users as if
it is a good practice to type (either from the command line or in their
scripts) "git-foo" dashed form, the goal is obviously good.
I however am not sure if posting a single patch that is 1300 lines long
would be the most effective way to achieve that goal. Splitting them into
a handful batches, spread over a few weeks to give reviewers some
breathing room, might make it more reviewable. I dunno.
Let's see hands; how many of you on git mailing list are willing to review
a re-rolled series, when Heikki posts one? How can we make it easier to
apply "many eyeballs" to make the issue shallower?
One thing you might be able to do would be to separate user visible
strings and in-comment strings. Frankly, when a comment talks about a git
subprogram, blindly replacing s/-/ / makes it _far worse_ and the
resulting text is harder to read for git hackers (who are primary
audiences of comments). E.g. taking one random example from your patch:
diff --git a/builtin-init-db.c b/builtin-init-db.c
index baf0d09..81aa822 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -37,7 +37,7 @@ static void copy_templates_1(char *path, int baselen,
/* Note: if ".git/hooks" file exists in the repository being
* re-initialized, /etc/core-git/templates/hooks/update would
- * cause git-init to fail here. I think this is sane but
+ * cause git init to fail here. I think this is sane but
* it means that the set of templates we ship by default, along
* with the way the namespace under .git/ is organized, should
* be really carefully chosen.
I do not think this is an improvement. We are talking about a single
subprogram, not what the user types from the command line. Either of
these two would be easier to read:
... would cause git-init to fail here...
... would cause "init" to fail here...
than:
... would cause git init to fail here...
Among these three, I think the second one is the most readable. Perhaps
dropping "git-" prefix when we are talking about a git subprogram, and
quoting the word to make it stand out, could be a good approach , than
replacing it with "git " prefix.
On Fri, Aug 29, 2008 at 07:12:18PM -0700, Junio C Hamano wrote:
I however am not sure if posting a single patch that is 1300 lines long
would be the most effective way to achieve that goal. Splitting them into
a handful batches, spread over a few weeks to give reviewers some
breathing room, might make it more reviewable. I dunno.
[CUT]
One thing you might be able to do would be to separate user visible
strings and in-comment strings.
[CUT]
... would cause git-init to fail here...
... would cause "init" to fail here...
than:
... would cause git init to fail here...
Among these three, I think the second one is the most readable.
Okay, will split patches into smaller ones, distribute them over time,
and use the second form for _in-comment_ strings. User visible strings
will say "git init".
--
Heikki Orsila
heikki.orsila@iki.fi
http://www.iki.fi/shd
Junio C Hamano wrote:
Heikki Orsila [off-list ref] writes:
quoted
On Fri, Aug 29, 2008 at 05:56:58PM -0700, Junio C Hamano wrote:
...
quoted
I do not think a single patch with the above hunk (which I think is a
mistake) and other bits that are obviously good (e.g. the first hunk to
builtin-apply.c we see below) is reviewable, but I cannot think of a
better alterantive. Sigh...
...
quoted
I did not look at the rest.
Is the concept OK for this change? I can submit another patch.
By "concept" if you mean the goal to stop implying to the end users as if
it is a good practice to type (either from the command line or in their
scripts) "git-foo" dashed form, the goal is obviously good.
I however am not sure if posting a single patch that is 1300 lines long
would be the most effective way to achieve that goal. Splitting them into
a handful batches, spread over a few weeks to give reviewers some
breathing room, might make it more reviewable. I dunno.
Let's see hands; how many of you on git mailing list are willing to review
a re-rolled series, when Heikki posts one? How can we make it easier to
apply "many eyeballs" to make the issue shallower?
I've started doing the exact same thing, so I'll happily review some.
I'm currently fiddling with the linkgit asciidoc macro. Since I know
nothing of asciidoc macro syntax, it's very slow going and immensely
fun :)
quoted hunk
One thing you might be able to do would be to separate user visible
strings and in-comment strings. Frankly, when a comment talks about a git
subprogram, blindly replacing s/-/ / makes it _far worse_ and the
resulting text is harder to read for git hackers (who are primary
audiences of comments). E.g. taking one random example from your patch:
diff --git a/builtin-init-db.c b/builtin-init-db.c
index baf0d09..81aa822 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -37,7 +37,7 @@ static void copy_templates_1(char *path, int baselen,
/* Note: if ".git/hooks" file exists in the repository being
* re-initialized, /etc/core-git/templates/hooks/update would
- * cause git-init to fail here. I think this is sane but
+ * cause git init to fail here. I think this is sane but
* it means that the set of templates we ship by default, along
* with the way the namespace under .git/ is organized, should
* be really carefully chosen.
I do not think this is an improvement. We are talking about a single
subprogram, not what the user types from the command line. Either of
these two would be easier to read:
... would cause git-init to fail here...
... would cause "init" to fail here...
than:
... would cause git init to fail here...
Among these three, I think the second one is the most readable. Perhaps
dropping "git-" prefix when we are talking about a git subprogram, and
quoting the word to make it stand out, could be a good approach , than
replacing it with "git " prefix.
I concur.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
Le samedi 30 août 2008, Junio C Hamano a écrit :
quoted hunk
One thing you might be able to do would be to separate user visible
strings and in-comment strings. Frankly, when a comment talks about a
git subprogram, blindly replacing s/-/ / makes it _far worse_ and the
resulting text is harder to read for git hackers (who are primary
audiences of comments). E.g. taking one random example from your patch:
diff --git a/builtin-init-db.c b/builtin-init-db.c
index baf0d09..81aa822 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -37,7 +37,7 @@ static void copy_templates_1(char *path, int baselen,
/* Note: if ".git/hooks" file exists in the repository being
* re-initialized, /etc/core-git/templates/hooks/update would
- * cause git-init to fail here. I think this is sane but
+ * cause git init to fail here. I think this is sane but
* it means that the set of templates we ship by default, along
* with the way the namespace under .git/ is organized, should
* be really carefully chosen.
I do not think this is an improvement. We are talking about a single
subprogram, not what the user types from the command line. Either of
these two would be easier to read:
... would cause git-init to fail here...
... would cause "init" to fail here...
I think "git init" is even better, as "init" could be ambiguous.
than:
... would cause git init to fail here...
Among these three, I think the second one is the most readable. Perhaps
dropping "git-" prefix when we are talking about a git subprogram, and
quoting the word to make it stand out, could be a good approach , than
replacing it with "git " prefix.
Regards,
Christian.