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

Don't use git based 3.0.1, bump the version since our fork has changes. #8422

Merged
merged 1 commit into from May 4, 2016

Conversation

jrafanie
Copy link
Member

@jrafanie jrafanie commented May 3, 2016

It's confusing to re-use a gem version with changes on top. We can bump our
version using a prerelease 3.0.2 version and not worry about colliding with
upstream since we won't be moving to theirs until 3.0.2 is released.

irb(main):031:0> Gem::Requirement.new("~> 3.0.2.alpha-miq.1").satisfied_by?(Gem::Version.new("3.0.2.alpha-miq.1"))
=> true
irb(main):032:0> Gem::Requirement.new("~> 3.0.2.alpha-miq.1").satisfied_by?(Gem::Version.new("3.0.2.alpha-miq.2"))
=> true
irb(main):033:0> Gem::Requirement.new("~> 3.0.2.alpha-miq.1").satisfied_by?(Gem::Version.new("3.0.2"))
=> true

Note, the a-z character version component is used for prereleased versions.
You therefore cannot try to use a released gem version, 3.0.1 and add a .miq or
something like "~> 3.0.1.miq-1` because the EXISTING 3.0.1 satisfies that
prerelease constraint:

irb(main):029:0> Gem::Requirement.new("~> 3.0.1.alpha-miq.1").satisfied_by?(Gem::Version.new("3.0.1"))
=> true

All that for rails 5 support from FooBarWidget default_value_for PR 57

@carbonin
Copy link
Member

carbonin commented May 3, 2016

👍 Looks good
@simaishi @isimluk

@Fryguy
Copy link
Member

Fryguy commented May 3, 2016

Is 3.0.2 expected to be the next version? I think for the other gems, we did something like 3.0.1.miq-1 to show that it's really 3.0.1 plus stuff

@Fryguy
Copy link
Member

Fryguy commented May 3, 2016

Also, do we have to do this for all of the git based gems, since I'd assume they'd all hit the same problem.

@jrafanie
Copy link
Member Author

jrafanie commented May 3, 2016

@simaishi will going from "3.0.2.pre" to "3.0.2" when it is released be enough of a version change to cause the git based gem / released gem paths to be changed? I think so.

@jrafanie
Copy link
Member Author

jrafanie commented May 3, 2016

Is 3.0.2 expected to be the next version? I think for the other gems, we did something like 3.0.1.miq-1 to show that it's really 3.0.1 plus stuff

I agree but we can't for this reason:

irb(main):029:0> Gem::Requirement.new("~> 3.0.1.miq-1").satisfied_by?(Gem::Version.new("3.0.1"))
=> true

@jrafanie
Copy link
Member Author

jrafanie commented May 3, 2016

Good point @Fryguy, I'll update the description why we can't use 3.0.1.miq1

@Fryguy
Copy link
Member

Fryguy commented May 3, 2016

Ohhh....because that string at the end usually represent "prerelease", not "postrelease"....ok, then yeah, we have no choice.

@carbonin
Copy link
Member

carbonin commented May 3, 2016

This is only an issue for git based gems that used to be released gems.

So I think it is just this one and ovirt_metrics, but I think that one should be going back to released soon right @chrisarcand?

@Fryguy
Copy link
Member

Fryguy commented May 3, 2016

@jrafanie I'm not sure "pre1" is the best string for us to choose. We may want something that won't possibly conflict with the gem's upstream decision to have their own prelease. Additionally, the digit should probably be separated by a dot as per SemVer: http://semver.org/#spec-item-9

So, I'm thinking "3.0.2-alphamiq.1 " which alphabetically puts it ahead of most anything that might be released (I think). What do you think?

@simaishi
Copy link
Contributor

simaishi commented May 3, 2016

@jrafanie I think so too, as long as it's not exactly same as what it was before, we should be ok.

@jrafanie
Copy link
Member Author

jrafanie commented May 3, 2016

@jrafanie I'm not sure "pre1" is the best string for us to choose. We may want something that won't possibly conflict with the gem's upstream decision to have their own prelease. Additionally, the digit should probably be separated by a dot as per SemVer: http://semver.org/#spec-item-9

So, I'm thinking "3.0.2.alpha-miq.1 " which alphabetically puts it ahead of most anything that might be released (I think). What do you think?

@Fryguy Naming is hard, so I used what rails uses.

3.0.2.alpha-miq.1 works so I'll use it.

irb(main):031:0> Gem::Requirement.new("~> 3.0.2.alpha-miq.1").satisfied_by?(Gem::Version.new("3.0.2.alpha-miq.1"))
=> true
irb(main):032:0> Gem::Requirement.new("~> 3.0.2.alpha-miq.1").satisfied_by?(Gem::Version.new("3.0.2.alpha-miq.2"))
=> true
irb(main):033:0> Gem::Requirement.new("~> 3.0.2.alpha-miq.1").satisfied_by?(Gem::Version.new("3.0.2"))
=> true

https://bugzilla.redhat.com/show_bug.cgi?id=1328908

It's confusing to re-use a gem version with changes on top.  We can bump our
version using a prerelease 3.0.2 version and not worry about colliding with
upstream since we won't be moving to theirs until 3.0.2 is released.

```ruby
irb(main):031:0> Gem::Requirement.new("~> 3.0.2.alpha-miq.1").satisfied_by?(Gem::Version.new("3.0.2.alpha-miq.1"))
=> true
irb(main):032:0> Gem::Requirement.new("~> 3.0.2.alpha-miq.1").satisfied_by?(Gem::Version.new("3.0.2.alpha-miq.2"))
=> true
irb(main):033:0> Gem::Requirement.new("~> 3.0.2.alpha-miq.1").satisfied_by?(Gem::Version.new("3.0.2"))
=> true
```

Note, the a-z character version component is used for prereleased versions.
You therefore cannot try to use a released gem version, 3.0.1 and add a .miq or
something like "~> 3.0.1.miq-1` because the EXISTING 3.0.1 satisfies that
prerelease constraint:

```ruby
irb(main):029:0> Gem::Requirement.new("~> 3.0.1.alpha-miq.1").satisfied_by?(Gem::Version.new("3.0.1"))
=> true
```

All that for rails 5 support from FooBarWidget default_value_for PR 57
@jrafanie
Copy link
Member Author

jrafanie commented May 3, 2016

@Fryguy updated

@miq-bot
Copy link
Member

miq-bot commented May 3, 2016

<github_pr_commenter_batch />Some comments on commit jrafanie@d05d267

@miq-bot
Copy link
Member

miq-bot commented May 3, 2016

Checked commit jrafanie@d05d267 with ruby 2.2.3, rubocop 0.37.2, and haml-lint 0.16.1
1 file checked, 1 offense detected

Gemfile

@chrisarcand
Copy link
Member

chrisarcand commented May 3, 2016

@carbonin Yup, thanks @Fryguy

My two cents here is that I wouldn't bother wasting too much time debating about an unofficial prerelease name; Personally I assume that if a gem uses a specific fork and branch (especially), then there are surely specific changes there (I don't need a prerelease name to clue me off any further). But I understand why you'd prefer it anyway 🙇

@jrafanie
Copy link
Member Author

jrafanie commented May 3, 2016

My two cents here is that I wouldn't bother wasting too much time debating about an unofficial prerelease name; Personally I assume that if a gem uses a specific fork and branch (especially), then there are surely specific changes there (I don't need a prerelease name to clue me off any further). But I understand why you'd prefer it anyway 🙇

@chrisarcand I agree, the important thing is 3.0.1 + patches cannot remain as 3.0.1 since it's confusing to most of us and various tooling. If we require our patches on top of 3.0.1, we must use the next patch release + some prerelease value such as ~> 3.0.2.alpha-miq.1 or just 3.0.2.pre1.

@chessbyte chessbyte merged commit 6ad9841 into ManageIQ:master May 4, 2016
@chessbyte chessbyte added this to the Sprint 40 Ending May 9, 2016 milestone May 4, 2016
chessbyte added a commit that referenced this pull request May 4, 2016
Don't use git based 3.0.1, bump the version since our fork has changes.
(cherry picked from commit 6ad9841)
@jrafanie jrafanie deleted the bump_default_value_for branch May 6, 2016 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants