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

Why do you use custom version type? #4

Open
dtretyakov opened this issue Apr 16, 2014 · 11 comments
Open

Why do you use custom version type? #4

dtretyakov opened this issue Apr 16, 2014 · 11 comments

Comments

@dtretyakov
Copy link
Contributor

Hi, in FCL exists Version class: http://msdn.microsoft.com/en-us/library/system.version.aspx
Could you explain why do use use custom MigrationVersion type?

@ryanohs
Copy link
Collaborator

ryanohs commented Apr 16, 2014

I agree that the FCL Version class looks like a good substitute. Perhaps @g0t4 was avoiding using a sealed class, but he probably just didn't know about the Version class.

@g0t4
Copy link
Collaborator

g0t4 commented Apr 16, 2014

Is there something that MigrationVersion doesn't do that you need it to?

On Wed, Apr 16, 2014 at 11:20 AM, Ryan Ohs [email protected] wrote:

I agree that the FCL Version class looks like a good substitute. Perhaps
@g0t4 https://github.com/g0t4 was avoiding using a sealed class, but he
probably just didn't know about the Version class.


Reply to this email directly or view it on GitHubhttps://github.com//issues/4#issuecomment-40633514
.

@dtretyakov
Copy link
Contributor Author

I've made some changes in the following pull request: #5

And while looking through project I found that exists MigrationVersion class with a custom BSON serializer. After replacing of it by Version class nothing has changes, so It was interesting for me for a which purposes it was used.

May be after review of this changes you can publish a new version in the nuget?

@g0t4
Copy link
Collaborator

g0t4 commented Apr 16, 2014

Can we split these out into separate commits, changing Version isn't a
necessity at this point and requires further thinking/testing and I don't
see any value out of the conversion at this point

On Wed, Apr 16, 2014 at 11:50 AM, Dmitry Tretyakov <[email protected]

wrote:

I've made some changes in the following pull request: #5#5

And while looking through project I found that exists MigrationVersionclass with a custom BSON serializer. After replacing of it by
Version class nothing has changes, so It was interesting for me for a
which purposes it was used.

May be after review of this changes you can publish a new version in the
nuget?


Reply to this email directly or view it on GitHubhttps://github.com//issues/4#issuecomment-40637081
.

@dtretyakov
Copy link
Contributor Author

Our concern: MigrationVersion provides ability to use up to 3 version numbers, but we're using all of 4 numbers, which available in the Version class.

I'll split pull request in two parts. If you worry about backward compatibility then both of CollectionMigration and Migration classes accept version numbers as a string:

@dtretyakov
Copy link
Contributor Author

So you can look at the following:

@g0t4
Copy link
Collaborator

g0t4 commented Apr 16, 2014

Awesome, that seems reasonable to me for 4 numbers

On Wed, Apr 16, 2014 at 12:41 PM, Dmitry Tretyakov <[email protected]

wrote:

So you can look at the following:


Reply to this email directly or view it on GitHubhttps://github.com//issues/4#issuecomment-40642883
.

@g0t4
Copy link
Collaborator

g0t4 commented Apr 16, 2014

And FWIW I think this library's approach to migrations was a good starting
point, but it was based too much on the process of doing SQL migrations and
too coupled to .net (for no real reason)... going forward I will be working
on something that leverages mongo shell scripts in an organized fashion, no
reason to couple to the c# driver long term

On Wed, Apr 16, 2014 at 1:38 PM, Wes McClure [email protected] wrote:

Awesome, that seems reasonable to me for 4 numbers

On Wed, Apr 16, 2014 at 12:41 PM, Dmitry Tretyakov <
[email protected]> wrote:

So you can look at the following:


Reply to this email directly or view it on GitHubhttps://github.com//issues/4#issuecomment-40642883
.

@dtretyakov
Copy link
Contributor Author

Great, we're looking forward to look at the new migration toolkit.

@g0t4
Copy link
Collaborator

g0t4 commented Apr 17, 2014

Hey I'm just curious, what do you use 4 numbers for that can't be accomplished with 3?

@dtretyakov
Copy link
Contributor Author

We're using migration version associated with a complete product version.

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

No branches or pull requests

3 participants