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

Tag NamedTuples v3.0.3 [https://github.com/blackrock/NamedTuples.jl] #9525

Merged
merged 1 commit into from
Jun 10, 2017

Conversation

mdcfrancis
Copy link
Contributor

@mdcfrancis
Copy link
Contributor Author

Added the v3.0.4 tag as well to change set

@tkelman
Copy link
Contributor

tkelman commented May 26, 2017

Looks like this would cause some problems downstream with IterableTables - we may want to upper-bound the existing versions there, if return types changed in a breaking way

@shashi
Copy link
Contributor

shashi commented May 26, 2017

Making the upper bound change in this repository would suffice for now, right?

Possibly something to automate in the future.

@tkelman
Copy link
Contributor

tkelman commented May 26, 2017

yes, in-place in the metadata copies of the requires files. don't touch the existing sha1 files. can be a bit complicated, but we should add some helper scripts or functions to handle simple cases, yes

@davidanthoff
Copy link
Contributor

I'm looking into the breaking change now, but minimally the breaking change should not be handled via a patch version increase, right?

@davidanthoff
Copy link
Contributor

Alright, I looked into this now. The breaking change is fine for IterableTables and Query, in fact the new behavior makes things slightly easier on my end.

But versioning wise that change really should be v4.0.0, it is after all a breaking API change. So I think v3.0.3 should be tagged as is right now. But what is being proposed as v3.0.4 currently should instead be v4.0.0.

I'll add upper bounds on the currently released IterableTables and Query versions that exclude v4.0.0, and then I'll tag a new release of IterableTables and Query that works with NamedTuples v4.0.0.

@davidanthoff
Copy link
Contributor

Once #9567 is tagged it should be safe to tag the breaking change in NamedTuples as v4.0.0.

@shashi
Copy link
Contributor

shashi commented May 30, 2017

It seems like the breakage was caused by use of something not exported (NamedTuples.make_tuple - used to return a symbol, now returns the type itself).

As far as users who don't care about the guts of the package are concerned, this release adds a method to Base.isless and fixes problems in serializing NamedTuples. It's not a breaking release.

Can we merge this?

@mdcfrancis do you know about https://github.com/attobot/attobot ? It makes releasing / re-tagging a breeze. Thanks for your effort!

@davidanthoff
Copy link
Contributor

Well, it is breaking both IterableTables and Query, two of the most heavy users of the package. Yes, both packages are using the unexported make_tuple function, but that is the only way to make those packages work. Both packages have a lot of users, so this would cause lots of pain for lots of folks if this gets merged right now.

I did put everything in place so that a NamedTuples v4.0.0 with these changes would not cause any trouble downstream. If you don't want to increase this to v4.0.0 I would have to change all of these limits again so that things don't break. Happy to do that, but not sure I'll be able to fit that in tomorrow, so that might take longer.

@shashi
Copy link
Contributor

shashi commented May 30, 2017

I did put everything in place so that a NamedTuples v4.0.0

Ah I see. Then it might be easy to just tag v4.0.0 here

The only valid reason (IMHO :-P) to tag a major / minor release is if you're going to then fix up the old code with patch releases, this doesn't seem like the case here.

@davidanthoff
Copy link
Contributor

We've had a lot of patch releases with NamedTuples, including for old versions (e.g. v2.1.0).

I also second the attobot idea. Also, might it make sense to give some other folks commit access to the repository so that we can speed up things like tagging releases etc? @mdcfrancis?

@tkelman
Copy link
Contributor

tkelman commented May 30, 2017

It's better to leave too much space for retroactive patch releases than to leave too little space. Since the change in return type (even if the method was not exported) did cause noticeable problems downstream, it could be potentially valuable down the line to do a patch release that fixed some other bug but left in place the old return type behavior. Not saying that's needed right now, but it might be at some future time.

Would it be worth moving this package to an organization, such as JuliaCollections ?

@shashi
Copy link
Contributor

shashi commented May 30, 2017

One way to not need the version upper bound is to just use eval(:(@NT(x,y,z))) where you were using make_tuple([:x,:y,:z]) this seems more future proof anyway.

@davidanthoff
Copy link
Contributor

Bump. @mdcfrancis could you rename the v3.0.4 tag to v4.0.0 and leave the v3.0.3 tag as is?

Also, any chance you could give either @shashi or me commit access to NamedTuples.jl, so that we can move faster with situations like this?

@shashi
Copy link
Contributor

shashi commented Jun 8, 2017

I think v4.0.0 should also include JuliaData/NamedTuples.jl#36 and JuliaData/NamedTuples.jl#37

@mdcfrancis
Copy link
Contributor Author

I have reverted the v3.0.4 tag and will push a new v4.0.0 with the 3.0.4 code and the other two pull requests. Sorry for the delay.

@tkelman
Copy link
Contributor

tkelman commented Jun 10, 2017

closing in favor of #9753 then

@tkelman tkelman closed this Jun 10, 2017
@mdcfrancis
Copy link
Contributor Author

mdcfrancis commented Jun 10, 2017

Per the thread above we need the v3.0.3 tag that is in this, this merge request had two versions originally

@tkelman tkelman reopened this Jun 10, 2017
@tkelman tkelman merged commit 1768e0b into JuliaLang:metadata-v2 Jun 10, 2017
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