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

allow all bits values as type parameters. closes #6081 #9161

Merged
merged 1 commit into from
Dec 5, 2014

Conversation

timholy
Copy link
Member

@timholy timholy commented Nov 26, 2014

This backports b429303. Any reason not to do this?

@vtjnash vtjnash added this to the 0.3.4 milestone Nov 26, 2014
@ivarne
Copy link
Member

ivarne commented Nov 26, 2014

This would be good in my opinion, but others have argued that we shouldn't backport "Features" and APIs.

I think we need to create a written backporting policy, so that we have some guidance (other than gut feeling) to decide what should be backported, and what users can expect when they upgrade/downgrade between patchlevel releases.

@timholy
Copy link
Member Author

timholy commented Nov 26, 2014

If you're referring to the discussions about backporting the rand work, this seems like a pretty different category. A change in argument order could break code; this change seemingly couldn't break anything, unless someone is counting on something not working.

@MikeInnes
Copy link
Member

Luckily, we don't have too many corporate users yet.

if try Array{Float64, 1.5}; true end
  rand(a, b)
else
  rand(b, a)
end

@garborg
Copy link
Contributor

garborg commented Nov 26, 2014

Except in the downgrade direction, or the labs with varying patchlevel versions scenario that Viral mentioned. Haven't seen agreement -- maybe the important bit is just having a rule spelled out whether hard or soft.

@timholy
Copy link
Member Author

timholy commented Nov 26, 2014

@one-more-minute, that's a hoot. I'm definitely going to start using that code snippet in some of my other work :).

You're right with the multiple machine scenario. And of course, those of us who want it can maintain our own forks. So I'm neutral on whether this gets merged or not.

@tkelman
Copy link
Contributor

tkelman commented Nov 27, 2014

I'd be in favor of a written backporting policy, though neutral on this particular commit since I'm not sure what kind of bugs this fixes. What are the chances of this being used by accident by a package developer on 0.3.4, without realizing it won't work on 0.3.3 and below? Maybe that's unlikely, and there are things in 0.3.3 that fit in this category so not sure.

Also when cherry-picking commits for backporting, it helps to do git cherry-pick -x so the cross-reference sha ends up in the commit log too. At least with this one coming from a PR Github will now show which PR it came from which is nice, but that's harder to get from offline logs. This kind of tip should be put in a backporting policy/guide, whoever wants to write that.

@ViralBShah
Copy link
Member

We can always do major releases from master more frequently, by picking features that are ready to go, than introduce api changes in a point release. This would need a lot more discipline in introducing new features on master.

The policy we try to follow is what is defined by semver.org. Is that seen as too restrictive?

@ViralBShah
Copy link
Member

This is a tough one, since it is at least not api incompatible. I would be OK with merging this.

@garborg
Copy link
Contributor

garborg commented Nov 27, 2014

I thought either path is compliant regardless of API compatibility since
we're on 0.x.y (rule 4.).

On Wednesday, November 26, 2014, Viral B. Shah notifications@github.com
wrote:

We can always do major releases from master more frequently, by picking
features that are ready to go, than introduce api changes in a point
release. This would need a lot more discipline in introducing new features
on master.

The policy we try to follow is what is defined by semver.org. Is that
seen as too restrictive?


Reply to this email directly or view it on GitHub
#9161 (comment).

JeffBezanson added a commit that referenced this pull request Dec 5, 2014
allow all bits values as type parameters. closes #6081
@JeffBezanson JeffBezanson merged commit ebfc7da into release-0.3 Dec 5, 2014
@ivarne ivarne deleted the bitstypes branch December 5, 2014 18:41
simonster added a commit to JuliaData/DataFrames.jl that referenced this pull request Jan 6, 2015
@tkelman
Copy link
Contributor

tkelman commented Jan 21, 2015

I think this can be fixed retroactively because of the way METADATA works, but please, please, please, when you start using backported features like this in packages, be very explicit about your minimum Julia version dependency in REQUIRE. Ref JuliaIO/HDF5.jl#203 (comment)

@JeffBezanson
Copy link
Member

I slightly regret merging this. I'd really rather only merge unambiguous bug fixes.

@tkelman
Copy link
Contributor

tkelman commented Jan 22, 2015

Although we're trying to keep the 0.3.x updates frequent and painless, plenty of users are much further from the development action than we are and don't know that there are newer versions available or that they would have any reason to update just a few months after installing Julia for the first time. If they hit a legitimate bug that has been fixed that's one thing, but we should try not to accidentally break packages for those users who stay on the same 0.3.x version for a while.

/soapbox

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.

8 participants