New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix json comparison using Irmin.Type.equal #588

Merged
merged 5 commits into from Nov 8, 2018

Conversation

Projects
None yet
2 participants
@zshipko
Contributor

zshipko commented Nov 2, 2018

Fixes #587

| `O a, `O b ->
(try
List.for_all2 (fun (k, v) (k', v') ->
equal (List.assoc k b) v && equal (List.assoc k' a) v') a b

This comment has been minimized.

@samoht

samoht Nov 7, 2018

Member

The List.assoc looks a bit strange. Maybe:

(try List.for_all2 (fun (k, v) (k', v') -> k=k' && equal v v') (List.sort compare_fst a) (List.sort compare_snd b) 
with Invalid_argument _ -> false)

e.g. sort the lists first, to avoid a quadratic complexity with List.assoc

This comment has been minimized.

@zshipko

zshipko Nov 7, 2018

Contributor

Thanks, that is much nicer!

(try
List.for_all2 (fun (k, v) (k', v') ->
equal (List.assoc k b) v && equal (List.assoc k' a) v') a b
with Not_found | Invalid_argument _ -> false)
k = k && equal v v') (List.sort compare_fst a) (List.sort compare_fst b)

This comment has been minimized.

@samoht

samoht Nov 7, 2018

Member

k = k' I guess :-)

This comment has been minimized.

@zshipko

zshipko Nov 7, 2018

Contributor

Oops, fixed :-)

@samoht

This comment has been minimized.

Member

samoht commented Nov 8, 2018

I am not sure why the windows CI is unhappy (it seems to be stuck while synchronising pinned packages?), so merging.

@samoht samoht merged commit ffe7859 into mirage:master Nov 8, 2018

1 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@zshipko zshipko deleted the zshipko:fix-json-compare branch Nov 8, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment