[PATCH 1 of 4] histedit: make verification configurable

Durham Goode durham at fb.com
Thu Nov 12 23:52:55 CST 2015



On 11/12/15 5:45 PM, Mateusz Kwapich wrote:
> # HG changeset patch
> # User Mateusz Kwapich <mitrandir at fb.com>
> # Date 1447375340 28800
> #      Thu Nov 12 16:42:20 2015 -0800
> # Node ID 659c8c265d8516a5685ac226abc91aaa79028202
> # Parent  f9984f76fd90e439221425d751e29bae17bec995
> histedit: make verification configurable
>
> Before we can add a 'base' action to histedit need to change verification
> so that action can specify which steps of verification should run for it.
>
> Also it's everything we need for the exec and stop actions implementation.
>
> I thought about baking verification into each histedit action (so each
> of them is responsible for verifying its constraints) but it felt wrong
> because:
>   - every action would need to know its context (eg. the list of all other
>     actions)
>   - a lot of duplicated work will be added - each action will iterate through
>     all others
>   - the steps of the verification would need to be extracted and named anyway
>     in order to be reused
Mateusz and I talked about this in person, but I'll put my opinion here 
for broader awareness.

Instead of having one-verify-function-to-rule-them-all, I think a 
verify() function on each action is cleaner. For instance, having a 
verify function on each action would put the new 'base' verify logic 
(patch 2) right inside the 'base' code.

If we don't delegate the verification, you could imagine future actions 
of arbitrary complexity that would require changes to the core histedit 
code for verification.  For instance, if we had a 'pickrange' action 
(like "pickr X::Y"), it doesn't fit any of the existing conventions and 
would require packing yet more knowledge into the global verify() logic.

But I admit this complaint is pretty unimportant, and would be trivial 
to implement later if we change our minds.  So I'd rather see these 
patches get in than see my complaint addressed.


More information about the Mercurial-devel mailing list