From: Jonathan Nieder Date: Sun, 17 Jul 2011 18:43:19 GMT Subject: Re: [PATCH 09/17] revert: Don't create invalid replay_opts in parse_args Message-ID: <20110717184319.GE27787@elie> In-Reply-To: Ramkumar Ramachandra wrote: > Thanks. New commit message de-emphasizing "four": Looks good. While at it, it might be nice to simplify the final sentence: > 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".