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

Do not use items/layouts (mutable identifier) as hash key #1184

Merged
merged 1 commit into from Jun 13, 2017

Conversation

Projects
None yet
1 participant
@ddfreyne
Member

ddfreyne commented Jun 11, 2017

Document’s @identifier is mutable, and is therefore not a good candidate for use in a hash key.

  • Add test case
  • Remove invariant check

Fixes #1185.

@ddfreyne

This comment has been minimized.

Show comment
Hide comment
@ddfreyne

ddfreyne Jun 11, 2017

Member

Alternative idea: Let the directed graph only contain references, e.g. [:item, '/foo.md'].

Member

ddfreyne commented Jun 11, 2017

Alternative idea: Let the directed graph only contain references, e.g. [:item, '/foo.md'].

@ddfreyne

This comment has been minimized.

Show comment
Hide comment
@ddfreyne

ddfreyne Jun 11, 2017

Member

This breaks equality (two objects can be equal even if their hash is different)

Member

ddfreyne commented Jun 11, 2017

This breaks equality (two objects can be equal even if their hash is different)

@ddfreyne ddfreyne changed the title from Do not use identifier (mutable) as hash key to Do not use items/layouts (mutable identifier) as hash key Jun 12, 2017

@ddfreyne

This comment has been minimized.

Show comment
Hide comment
@ddfreyne

ddfreyne Jun 13, 2017

Member

The directed graph will now store only references, rather than objects themselves.

Member

ddfreyne commented Jun 13, 2017

The directed graph will now store only references, rather than objects themselves.

@ddfreyne ddfreyne merged commit 2f08e27 into master Jun 13, 2017

3 checks passed

codecov/patch 100% of diff hit (target 98.71%)
Details
codecov/project 98.71% (+<.01%) compared to c42b115
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ddfreyne ddfreyne deleted the mut-identifier-as-hash-key branch Jun 13, 2017

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