Re: [PATCH 09/17] revert: Don't create invalid replay_opts in parse_args

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jul 17, 2011, 18:43 UTC
Message-ID: <20110717184319.GE27787@elie>
In-Reply-To: <CALkWK0kcZi87watFTZCFUJpNf1T-wnRPyQ6FcDxmvFzAZE3osg@mail.gmail.com>

Ramkumar Ramachandra wrote:

> Thanks.  New commit message de-emphasizing "four":

Looks good. While at it, it might be nice to simplify the final sentence:

Show 14 quoted lines

> revert: Don't create invalid replay_opts in parse_args
>
> The "--ff" command-line option cannot be used with some other
> command-line options.  However, parse_args still parses these
> incompatible options into a replay_opts structure for use by the rest
> of the program.  Although pick_commits, the current gatekeeper to the
> cherry-pick machinery, checks the validity of the replay_opts
> structure before before starting its operation, there will be multiple
> entry points to the cherry-pick machinery in future.  To futureproof
> the code and catch these errors in one place, make sure that an
> invalid replay_opts structure is not created by parse_args in the
> first place.  Also ensure that regressions in maintaining this
> invariant are caught in the future by adding an assertion in
> pick_commits.

I think the idea is "we still check the options struct for validity in pick_commits, but this is an assert() call now to emphasize that it's the caller's responsibility to get it right".