[PATCH 2 of 3 V2] shelve: refactor option combination check to add new one easily

[Pierre-Yves David]

On 06/06/2014 06:24 AM, FUJIWARA Katsunori wrote:
> At Thu, 05 Jun 2014 18:59:06 -0500,
> Sean Farley wrote:
>>
>>
>> FUJIWARA Katsunori writes:
>>
>>> # HG changeset patch
>>> # User FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
>>> # Date 1401977395 -32400
>>> #      Thu Jun 05 23:09:55 2014 +0900
>>> # Node ID c90c042600bcb28108366b8c08a06d9ac3259ab7
>>> # Parent  804a341610d154c8a2a4d3d63e36ff2cf08acc4f
>>> shelve: refactor option combination check to add new one easily
>>>
>>> Before this patch, the name of the newly added option should be added
>>> into each strings to be passed to 'checkopt()' internal function:
>>> these are white-space-separated list of un-acceptable option names.
>>>
>>> The name of new option should be added into multiple strings, because
>>> every options can belong to only one category of 'create', 'cleanup',
>>> 'delete' or 'list'.
>>>
>>> In addition of this redundancy, each strings passed to 'checkopt()'
>>> are already too long to include new one.
>>>
>>> This patch refactors option combination check to add new one easily in
>>> succeeding patch.
>>>
>>> New 'checkopt()' takes only one of categories ('cleanup', 'delete' or
>>> 'list'), and checks whether option allowed only for other categories
>>> is specified or not, if specified category is activated in 'opts'.
>>>
>>> 'date' entry is listed in 'allowableopts', but commented out, because:
>>>
>>>    - 'date' shouldn't be checked for test
>>>
>>>      checking 'date' causes unexpected failure of 'test-shelve.t',
>>>      because 'run-test.py' puts "[default] shelve = --date '0 0'" into
>>>      hgrc.
>>>
>>>    - but explicit listing it up can advertise that this ignoring is
>>>      intentional
>>
>> Yes, this seems fine to me. If you want something to bikeshed (Katsunori
>> should verify this):
>
> Thank you for your refining my text, Sean.
>
> I can't find any problems out in refined one.
>
> Should I resend refined one again ? > Pierre-Yves

Yes please resend a the full fixed series.

Also please replace usage of "succeeding changeset" with "following 
changeset". "succeeding" has an evolution connotation. Those word appear 
a couple of time in your series.


-- 
Pierre-Yves David