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

Add possibility to unpersist data one generation younger #71

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kkafara
Copy link
Contributor

@kkafara kkafara commented Jul 5, 2017

#57
Allow Persister and Migrator with version V to unpersist data that have not only version X <= V but also V + 1.
Possible use-case scenario described in #57 (comment)

Allow `Persister` and `Migrator` with version `V`
to unpersist data that have not only version `X <= V`
but also `V + 1`
@kkafara
Copy link
Contributor Author

kkafara commented Jul 27, 2017

@agemooij did you manage to find few minutes to look at it?

@@ -36,40 +36,71 @@ package migrations {

/**
* A `Migrator[R, V]` can migrate raw values of type R from older
* versions to version `V` by applying a specific `Migration[R]` to it.
* versions to version `V` or from version one generation younger
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to use the word "newer" instead of "younger" to describe Versions with greater numbers than the older ones. It's just a vocabulary clarification, as we had misunderstanding discussing this.

@psliwa
Copy link
Contributor

psliwa commented Aug 7, 2017

@agemooij @raboof If you are ok, I also would prefer @kkafara's solution over #62, so if you merge this PR, my PR may be closed.

)
}

def backFrom[NextV <: Version : VersionInfo](migration: Migration[R])(implicit isNextAfter: IsNextVersionAfter[NextV, V]): Migrator[R, V] = {
Copy link

@mzywiol mzywiol Aug 10, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about renaming this method to andBackFrom? This way it will gramatically point out that this method should be called only once and at the end of the to method calls:
from[V1](...).to[V2](...).to[V3].andBackFrom[V4](...)
Or even:
from[V1](...).andBackFrom[V4](...)

While this:
from[V1](...).to[V2](...).andBackFrom[V3](...).to[V3](...)
and this:
from[V1](...).to[V2](...).andBackFrom[V3](...).andBackFrom[V4](...)
would automatically look suspicious in code and would be easier to catch in code reviews.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from[V1](...).to[V2](...).andBackFrom[V3](...).to[V3](...) - this is possible but will have no effect now

from[V1](...).to[V2](...).andBackFrom[V3](...).andBackFrom[V4](...) - this is impossible - will not compile in current version of code


/**
* Creates a JsonPersister[T, V] where V is a version greater than V1.
* It will always persist instances of T to version V but it will use the specified
* JsonMigrator[V] to migrate any values older than version V to version V before
* unpersisting them.
* JsonMigrator[V] to migrate any values older or or one generation younger
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: The word or is duplicated.

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