Skip to content
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

Implement Transaction#mutated? #310

Merged
merged 1 commit into from
Aug 15, 2016
Merged

Implement Transaction#mutated? #310

merged 1 commit into from
Aug 15, 2016

Conversation

no-reply
Copy link
Member

#mutated? must return true when the transaction would change the
repository if executed against it's state at transaction start time.
We allow implementations to return true in other cases, as well; there
is no guarantee that a false result will be returned in any specific
circumstances (or ever).

For the base transaction, we simply check whether @changes is
empty. This can give false positives, but avoids a worst-case linear
check.

For subclasses with no custom implementation, we raise a
NotImplementedError.

For the SerializableTransaction included in the default
Repository::Implementation, we use #equal? on the Hamster::Hash
instances. This will always give a correct response for the actual
implementation, moreover, it is guaranteed not to give false negatives
even if a user messes with the snapshot using #send.

Closes #308.

@coveralls
Copy link

coveralls commented Aug 12, 2016

Coverage Status

Coverage decreased (-0.01%) to 90.971% when pulling 729d251 on feature/tx-mutated into fc06d1f on develop.

@no-reply
Copy link
Member Author

Do we have a way to silence dependency-ci on wirble and convince it that rdf-isomorphic is maintained

@no-reply
Copy link
Member Author

Note the caveats about false positives.

I considered the possibility of using true and false return values only when status is guaranteed. A "truthy" (but not == true) value would be given where as false positive is possible (as in the base class).

This seems viable, but I opted for the simpler approach as a starting place.

@coveralls
Copy link

coveralls commented Aug 12, 2016

Coverage Status

Coverage decreased (-0.01%) to 90.971% when pulling 7a6ae88 on feature/tx-mutated into fc06d1f on develop.

@gkellogg
Copy link
Member

I think you got it right. Don't know about silencing depend nicest, but it's intended to make our lives easier, not harder. See what you can figure out.

@no-reply
Copy link
Member Author

What do you think about just dropping the wirble dependency? Do you use that in development?

I let libraries.io know through their form that rdf-isomorphic is still maintained. I'm kind of hoping they'll explain where they got the idea it wasn't.

@gkellogg
Copy link
Member

Dropping warble should be fine, it seems to be more of a pain than it's worth.

WRT rdf-isomorphic, probably because no recent releases, although there was a commit in July.

@no-reply no-reply force-pushed the feature/tx-mutated branch from 7a6ae88 to fe1a3f8 Compare August 15, 2016 15:19
@coveralls
Copy link

coveralls commented Aug 15, 2016

Coverage Status

Changes Unknown when pulling fe1a3f8 on feature/tx-mutated into * on develop*.

`#mutated?` must return `true` when the transaction would change the
repository if executed against it's state at transaction start time.
We allow implementations to return `true` in other cases, as well; there
is no guarantee that a `false` result will be returned in any specific
circumstances (or ever).

For the base transaction, we simply check whether `@changes` is
empty. This can give false positives, but avoids a worst-case linear
check.

For subclasses with no custom implementation, we raise a
`NotImplementedError`.

For the `SerializableTransaction` included in the default
`RepositoryImplementation`, we use `#equal?` on the `Hamster::Hash`
instances. This will always give a correct response for the actual
implementation, moreover, it is guaranteed not to give false negatives
even if a user messes with the snapshot using `#send`.
@no-reply no-reply force-pushed the feature/tx-mutated branch from fe1a3f8 to d94cd14 Compare August 15, 2016 15:38
@coveralls
Copy link

coveralls commented Aug 15, 2016

Coverage Status

Changes Unknown when pulling d94cd14 on feature/tx-mutated into * on develop*.

@no-reply no-reply merged commit 934db17 into develop Aug 15, 2016
@no-reply no-reply deleted the feature/tx-mutated branch August 15, 2016 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants