Re: [GSoC update] Sequencer for inclusion

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Jul 11, 2011, 17:57 UTC
Message-ID: <CALkWK0msdBdXX4oMkd+WAMR8PXTRT3ivjMrf3ZAMfBrsLoM=dQ@mail.gmail.com>
In-Reply-To: <20110711171713.GA5963@elie>

Hi Jonathan,

Jonathan Nieder writes:

Show 5 quoted lines

> Ramkumar Ramachandra wrote:
>> I'm excited to announce the first iteration of a fresh series
>  - has the result of applying each patch in the new iteration been
>   tested (for example by rebasing interactively with "exec make test"
>   after each "pick" line)?

Tests are still running.  I'll let you know the results in the morning.

>  - what changed since last time, for each patch?  (For the future, the
>   space under the "---" is generally a good place to put that
>   information.)

Will comment a little more on this in the morning. The main focus of this series is to showcase the new option parser, and show how it fits into the rest of the series.

>  - what proposed changes did not make it in, for each patch?  If any,
>   did they not fit well with the design, or was it more a matter of
>   "sure, that would be nice, but let's get this in first"?

A combination of both.  More on this later.

Show 18 quoted lines

>> I would have liked to reuse the gitconfig parser as-is for the opts
>> parsing, but it's too tangled up in config.c.  I think it's safe to
>> say that the opts file format deviates only slightly from the
>> gitconfig format, and I'm quite happy with the end result.
>
> To be precise, the format used includes
>
>        strategy-option = patience | renormalize
>
> to represent the effect of "-Xpatience -Xrenormalize".  My only worry
> about that is that the "|" can sound like "or", which would seem
> strange to a user that does not necessarily develop software (so is
> not thinking about bitfields).  The format used in config files puts
>
>        strategy-option = patience
>        strategy-option = renormalize
>
> as separate lines.

Okay, I can change to that if it's desirable. My rationale for using "|" is that lines like "key = value1" and "key = value2" tend to look odd -- it's like I'm reassigning the key a different value.

>> 4. New tests and documentation.  There's really no end to this
>
> Once each new feature has been documented and each new feature or
> fixed bug has an associated test, you've reached the end of this.

It depends on how rigorously you want to document and test things, no?
 For example, I haven't documented the formats of the configuration
files anywhere but in the commit messages.  Something in
Documentation/technical would be nice, but I think we should wait
until the format evolves a bit.  Since I haven't exposed anything like
a "--interactive" functionality, the user will never see it and we can
change it as and when we like.

Also, for things like the option parser, how far do you want to go with testing? How many kinds of malformed instruction sheets do you want to test with? I'll include some more basic tests soon, but I don't think we should go too deep, due to time constraints.

As you can see, I've included basic usage documentation. Do let me know what else is required right now.

Show 9 quoted lines

> My main worry is still the commit messages.  They don't need to be
> elaborate but they should explain the purpose and effect of each
> patch.  Part of the reason I care is that it makes the life of future
> readers using "git log -S" or "git bisect" before changing that code
> much easier.  Think of them like a special kind of comments that don't
> interfere with reading the code straight through.
>
> The main reason I care is that that information makes the code itself
> easier to review.

I completely agree. We should not compromise on commit messages -- they can't be fixed later.

Show 7 quoted lines

> I don't know how to move forward on that.  I can explain what's
> missing in each message, but I get the impression that you already
> understand that and there's something else (e.g., time) preventing
> them from getting fixed.  I could rewrite each commit message, but I
> am likely to miss things and come up with something that sounds
> vaguely plausible but doesn't accurately explain the intent.  What do
> you suggest?

I have updated many of the commit messages. Do let me know what's missing where. As usual, you can leave quick comments on IRC -- I'll check the logs in the morning.

Thanks!

-- Ram