Skip to content
This repository has been archived by the owner on Aug 12, 2022. It is now read-only.

Tag GLVisualize.jl v0.2.3 #8982

Closed
wants to merge 1 commit into from

Conversation

attobot
Copy link
Contributor

@attobot attobot commented Apr 24, 2017

Repository: JuliaGL/GLVisualize.jl
Release: v0.2.3
Diff: vs v0.2.2
requires vs v0.2.2:

--- 0.2.2/requires
+++ 0.2.3/requires
@@ -25,3 +25,5 @@
 Iterators
 Packing
 UnicodeFun
+
+Compat 0.18

cc: @SimonDanisch

Packing
UnicodeFun

Compat 0.18
Copy link
Contributor

Choose a reason for hiding this comment

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

this was already listed above but at 0.9.5, should modify the existing minimum instead of listing it twice

@attobot
Copy link
Contributor Author

attobot commented Apr 24, 2017

Repository: JuliaGL/GLVisualize.jl
Release: v0.2.3
Diff: vs v0.2.2
requires vs v0.2.2:

--- 0.2.2/requires
+++ 0.2.3/requires
@@ -3,7 +3,6 @@
 GLWindow 0.3
 GLAbstraction 0.3.1
 GeometryTypes 0.2
-Compat 0.9.5
 
 GLFW
 ModernGL
@@ -25,3 +24,5 @@
 Iterators
 Packing
 UnicodeFun
+
+Compat 0.18

cc: @SimonDanisch

@SimonDanisch
Copy link
Contributor

Woops...

@SimonDanisch
Copy link
Contributor

seems like I need to restrict the reactive version!

@attobot
Copy link
Contributor Author

attobot commented Apr 25, 2017

Repository: JuliaGL/GLVisualize.jl
Release: v0.2.3
Diff: vs v0.2.2
requires vs v0.2.2:

--- 0.2.2/requires
+++ 0.2.3/requires
@@ -3,12 +3,11 @@
 GLWindow 0.3
 GLAbstraction 0.3.1
 GeometryTypes 0.2
-Compat 0.9.5
 
 GLFW
 ModernGL
 FixedSizeArrays
-Reactive
+Reactive 0.3.3 0.3.7
 
 FixedPointNumbers
 ColorVectorSpace
@@ -25,3 +24,5 @@
 Iterators
 Packing
 UnicodeFun
+
+Compat 0.18

cc: @SimonDanisch

GLFW
ModernGL
FixedSizeArrays
Reactive 0.3.3 0.3.7
Copy link
Contributor

Choose a reason for hiding this comment

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

does this upper bound need to apply to all past versions?

Copy link
Contributor

Choose a reason for hiding this comment

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

What do you mean? But I think the range is indeed wrong, since it won't include 0.3.7, no?
GLVisualize works with 0.3.3 - 0.3.7 right now...

Copy link
Contributor

Choose a reason for hiding this comment

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

Upper bounds are exclusive. And what I mean is if you have an upper bound only on this newest version of GLVisualize, Pkg would be allowed to keep GLVisualize held back to 0.2.2 or earlier and upgrade Reactive to the latest. Would things work if it did that?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh... No it wouldn't.

Copy link
Contributor

Choose a reason for hiding this comment

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

What can be done about this?

Copy link
Contributor

Choose a reason for hiding this comment

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

If the latest Reactive version broke all past tags of GLVisualize, then best thing to do is open a separate PR that adds Reactive upper bounds to all the past GLVisualize tags' require files. Don't touch the sha or the git tags, just the metadata copies of the require files.

Copy link
Contributor

Choose a reason for hiding this comment

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

There was so much restructuring going on with Reactive, that I didn't realize that it was simply a bug: JuliaGizmos/Reactive.jl#136
So I guess this moves the problem to tagging reactive? We need to switch out the bugged tag with the bugfix tag?

Copy link
Contributor

@tkelman tkelman Apr 28, 2017

Choose a reason for hiding this comment

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

If prior versions work, and newer versions are going to work after the bugfix gets tagged, then I don't think you should have stricter bounds for it. You would want to avoid the particular single bugged version, and that is possible to express with bounds but it's a bit unintuitive and not many people have been doing disjoint ranges.

Copy link
Contributor

Choose a reason for hiding this comment

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

that's why I asked if it's appropriate to just rewire Reactive's tag, so that the bugged version isn't in METADATA anymore...

Copy link
Contributor

Choose a reason for hiding this comment

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

The buggy tag has been published for a while now, it should be superseded with a new tag but not modified in-place.

@attobot
Copy link
Contributor Author

attobot commented May 2, 2017

Repository: JuliaGL/GLVisualize.jl
Release: v0.2.3
Diff: vs v0.2.2
requires vs v0.2.2:

--- 0.2.2/requires
+++ 0.2.3/requires
@@ -3,12 +3,11 @@
 GLWindow 0.3
 GLAbstraction 0.3.1
 GeometryTypes 0.2
-Compat 0.9.5
 
 GLFW
 ModernGL
 FixedSizeArrays
-Reactive
+Reactive 0.4.1
 
 FixedPointNumbers
 ColorVectorSpace
@@ -25,3 +24,5 @@
 Iterators
 Packing
 UnicodeFun
+
+Compat 0.18

cc: @SimonDanisch

@SimonDanisch
Copy link
Contributor

Okay, so there shouldn't be any harm to merge this...
Now I need to open a PR to change the other tags as well, right?
How far should I go back?

GLFW
ModernGL
FixedSizeArrays
Reactive 0.4.1
Copy link
Contributor

Choose a reason for hiding this comment

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

do 0.3.x versions work?

If prior versions work, and newer versions are going to work after the bugfix gets tagged, then I don't think you should have stricter bounds for it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not all!

Copy link
Contributor

Choose a reason for hiding this comment

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

what's the minimum version then? what feature are you relying on? or bugfix before which none of the releases would work?

Copy link
Contributor

Choose a reason for hiding this comment

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

Bug got introduced in 3.7, and got fixed in 4.1 - no version is working with the bug

Copy link
Contributor

Choose a reason for hiding this comment

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

If there are versions prior to 0.3.7 that would work, then you shouldn't rule out installation of them.

@tkelman
Copy link
Contributor

tkelman commented May 2, 2017

Please do not do git checkout on other packages in your runtests.jl script. And if you do using AxisArrays, ImageAxes then you should depend on them directly.

@attobot
Copy link
Contributor Author

attobot commented May 2, 2017

Repository: JuliaGL/GLVisualize.jl
Release: v0.2.3
Diff: vs v0.2.2
requires vs v0.2.2:

--- 0.2.2/requires
+++ 0.2.3/requires
@@ -3,12 +3,11 @@
 GLWindow 0.3
 GLAbstraction 0.3.1
 GeometryTypes 0.2
-Compat 0.9.5
 
 GLFW
 ModernGL
 FixedSizeArrays
-Reactive
+Reactive 0.4.1
 
 FixedPointNumbers
 ColorVectorSpace
@@ -25,3 +24,5 @@
 Iterators
 Packing
 UnicodeFun
+
+Compat 0.18

cc: @SimonDanisch

@SimonDanisch
Copy link
Contributor

@timholy supplied that code.. I believe the rational behind it was, that these packages are not needed for the old images, but when Images uses them, it will also require them.
Not sure if putting them directly in the REQUIRE will mess with the old images

@tkelman
Copy link
Contributor

tkelman commented May 2, 2017

when Images uses them, it will also require them

That's only enforceable if it's guaranteed that all allowed Images versions (will always) do so. With no upper bound, there's no way to ensure that.

@SimonDanisch
Copy link
Contributor

Well, it doesn't need to, since all code that is using this is in a similar conditional. I believe @timholy used this as a switch to verify if one is one the new version of Images that works with those packages.

@MikeInnes
Copy link
Member

Please check that:

  • The package's CI is green for this tag (if applicable).
  • The version bounds are up to date.

Then reply with +ready so I can merge.

@tkelman
Copy link
Contributor

tkelman commented May 13, 2017

specifically w.r.t Reactive,

If there are versions prior to 0.3.7 that would work, then you shouldn't rule out installation of them.

@SimonDanisch
Copy link
Contributor

So how do I do a version range from 0.3.0 - 0.3.7 && 0.4.0 ?

@tkelman
Copy link
Contributor

tkelman commented May 19, 2017

Reactive 0.3.0 0.3.7 0.4.0

@SimonDanisch
Copy link
Contributor

Thanks! ... Too easy. This is actually even pretty clearly stated in the docs...

@SimonDanisch
Copy link
Contributor

should be closed in favor of: #9436

@tkelman tkelman closed this May 22, 2017
@attobot attobot deleted the GLVisualize/v0.2.3 branch May 22, 2017 09:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants