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

Adds updateAlertType support for apps using only major and minor numb… #21

Merged
merged 5 commits into from
Jun 1, 2015

Conversation

liebeskind
Copy link
Contributor

…ering system

@getaaron
Copy link
Contributor

getaaron commented Jun 1, 2015

I think an approach like this would be simpler:

if 2...3 ~= oldVersion.count && oldVersion.count == newVersion.count {
    if newVersion[0] > oldVersion[0] { // A.b[.c]
        alertType = majorUpdateAlertType
    } else if newVersion[1] > oldVersion[1] { // a.B[.c]
        alertType = minorUpdateAlertType
    } else if newVersion.count == 3 && newVersion[2] > oldVersion[2] { // a.b.C
        alertType = patchUpdateAlertType
    }
}

Although even this makes me want to refactor a HarpyVersion struct. :)

@ArtSabintsev
Copy link
Owner

Agreed. We should extend it to 4 as well, as ice seen many projects use that system as well. Want to add the code and push it live on here and cocoapods, Aaron?

@getaaron
Copy link
Contributor

getaaron commented Jun 1, 2015

I can get to it tonight unless @liebeskind wants to do it.

@ArtSabintsev
Copy link
Owner

Ok np - I may have time tonight as well - depends on go fast I get to my destination right now :) make a post here if you start working in it, so no one else does double work.

Thanks!

@liebeskind
Copy link
Contributor Author

Thanks for the feedback guys. I'll implement your suggestions now and re-pull.

DL

…be shown based on version updates. Also adds additional minor update - revision. Siren should now work if using a.b.c.d versioning system.
@ArtSabintsev
Copy link
Owner

Sounds go, @liebeskind! If you can make it handle from 2-4 numbers, that would be awesome.

@liebeskind
Copy link
Contributor Author

Should be good to go

@ArtSabintsev
Copy link
Owner

This looks great! Thanks! I'll accept the PR momentarily.

ArtSabintsev added a commit that referenced this pull request Jun 1, 2015
Adds AlertType support for apps using revision version numbering and for apps using online major and minor version numbering.
@ArtSabintsev ArtSabintsev merged commit 573ed1e into ArtSabintsev:master Jun 1, 2015
@ArtSabintsev
Copy link
Owner

ok, v0.4.1 is live. Changes to README and CONTRIBUTORS.md were also made.

Thanks again guys!

@getaaron
Copy link
Contributor

getaaron commented Jun 2, 2015

Thanks for the PR, @liebeskind.

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