Re: [PATCH] Make list of features auto-managed.

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 21, 2007, 09:18 UTC
Message-ID: <7vwsxxwtrh.fsf@assigned-by-dhcp.pobox.com>
In-Reply-To: <7vhcp1y954.fsf_-_@assigned-by-dhcp.pobox.com>

I am now getting sort of convinced that the list-features is a slippery slop towards total suckage, unless managed very carefully.

The "auto manage" patch I am responding to would tempt anybody to extend the mechanism to do something like the attached patch, because a one-line comment near the code, like:

	/* FEATURE<oneline-first-paragraph> */

does not look very useful nor pretty by itself in the source code. It makes it very tempting to actually describe the "feature". People with only half a brain would even advocate updating supported_features[] to a tuple of (name, explanation), and have "git version --list-features" to spit both out, or something like that. I really do not think we would want to go there.

A few years down the road, do we really care if we did not do something long time ago, but the code was updated and added as a new feature? That information belongs to the commit log, not to in-source comments. This will lead to the same mistake often made by users of other SCMs, embedding "$Log$" in their sources.

=== sample diff that is not acceptable ===

diff --git a/commit.c b/commit.c
index dbb28b5..4b713b6 100644
--- a/commit.c
+++ b/commit.c
@@ -1016,6 +1016,12 @@ static void pp_header(enum cmit_fmt fmt,
 	}
 }
 
+/*
+ * FEATURE<oneline-first-paragraph>: we used to treat the first line of the
+ * commit messages special, but messages imported from foreign culture often
+ * have more than one line in the first "paragraph".  We now treat the
+ * first paragraph as special for the purposes of --pretty=oneline.
+ */
 static void pp_title_line(enum cmit_fmt fmt,
 			  const char **msg_p,
 			  unsigned long *len_p,
@@ -1140,6 +1146,11 @@ static void pp_remainder(enum cmit_fmt fmt,
 	}
 }
 
+/*
+ * FEATURE<log-unlimited-message>: we used to have a hard limit
+ * of 16kB message, but that is sometimes not enough for huge commits
+ * imported from foreign culture.  We now dynamically grow the buffer.
+ */
 unsigned long pretty_print_commit(enum cmit_fmt fmt,
 				  const struct commit *commit,
 				  unsigned long len,