Skip to content

Make a v2.1 release? #726

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

Closed
dean0x7d opened this issue Mar 13, 2017 · 22 comments
Closed

Make a v2.1 release? #726

dean0x7d opened this issue Mar 13, 2017 · 22 comments
Milestone

Comments

@dean0x7d
Copy link
Member

Seems like there is a decent amount of new features and bug fixes for a v2.1 release. This should also be good timing with the recent release of Visual Studio 2017 and the related fixes in pybind11.

Bugs: Are there any critical bugs that should be fixed? Looking at the issues, there don't seem to be any showstoppers.

Features: Of the feature PRs, #488 and #693 are close to ready. I'd say #693 would be best to merge right at the beginning of v2.2.dev so it gets some time in master due to the low-level core changes, rather than going straight to release. #488 doesn't affect the core library in that way, but perhaps a general feature freeze for v2.1 would be best.

Thoughts?

@jagerman
Copy link
Member

Sounds good to me. I think current master is in pretty good shape, and I agree that #693 (or anything of a similar scope) should be in master for a while before a release. For non-core things like #488, I think we can accept them if they are ready to go in the next couple days.

@jagerman jagerman added this to the v2.1 milestone Mar 14, 2017
@jagerman
Copy link
Member

#488 is merged now; I also created v2.1 and v2.2 milestones to help categorize issues/PRs.

@jagerman
Copy link
Member

I think #671 will be an easy fix, as will adding compiler version checks to close #706; I've tagged them both for v2.1, and will submit PRs tomorrow.

@wjakob
Copy link
Member

wjakob commented Mar 14, 2017

Creating a v2.1 with the current master sounds good to me. I'll wait for @jagerman's two commits before pushing it out (and perhaps add 1-2 days of delay to be on the safe side?).

@wjakob
Copy link
Member

wjakob commented Mar 14, 2017

https://travis-ci.org/pybind/pybind11/jobs/210795174 :( -- I sense nondeterminism.

@MathMagique
Copy link
Contributor

A 2.1 release would be awesome, since I would love to use the (seemingly new) support for u16string. Thanks for the great work!

@jagerman
Copy link
Member

@wjakob that looks like a pypy bug: they just merged a fix for the tp_bases-is-incomplete issue, but when I tried removing the workaround from the MI PR I got a segfault in the same place. I'm going to investigate and report upstream (assuming it's the same issue).

@jagerman
Copy link
Member

jagerman commented Mar 14, 2017

The segfault issue is fixed in PyPy.

Edit: confirmed.

@dean0x7d
Copy link
Member Author

@jagerman What do you think about bumping #671 to v2.2? It's not a bug fix and it seems like there's still some discussion on performance going on there.

In that case, there would be just #727 left to merge and we can call it a release.

@jagerman
Copy link
Member

I think #730 is ready to merge, if you want to give it a quick review. There may (or may not!) be other optimizations we can make eventually for #671, but the dropping the forced copying for anything other than a c-contiguous array in #730 seems worthwhile regardless.

@jagerman
Copy link
Member

(The diff in #730 looks bigger than it really is: it's mostly style changes to better help me understand exactly what the code was doing.)

@dean0x7d
Copy link
Member Author

It might be better if @wjakob takes a look, if he has time. You'd definitely get a better review since I'm not familiar with that part of the code at all (on the "how it works" dev side, but also on the "what it does" user side as I haven't had opportunity to use py::vectorize at all).

@jagerman
Copy link
Member

jagerman commented Mar 18, 2017

Okay. As for #727, are we decided that requiring 2015u3 at a minimum is the way to go? I didn't get any answer to my request for a stronger justification for 2015u2 support.

Edit: a possible option is to relax the compile-time check to 2015u2, but keep our official, documented requirement at 2015u3.

@dean0x7d
Copy link
Member Author

dean0x7d commented Mar 18, 2017

As for #727, are we decided that requiring 2015u3 at a minimum is the way to go?

Yeah, I'd say it's the way to go. The arguments have been made in #727 and I don't think there is a practical way to maintain support for Update 2 and not enough justification to jump through the hoops.

Edit: Added context.

@jagerman
Copy link
Member

Yeah, I'd say it's the way to go.

The hard requirement of 2015u3, or allowing 2015u2 to compile, but only supporting 2015u3+?

@jagerman
Copy link
Member

Actually, no, never mind, the soft requirement won't work: 2015u2 won't work at all (with the conjunction/disjunction use in common.h).

@dean0x7d
Copy link
Member Author

Yeah, hard requirement (sorry, didn't see the edit). Adding support for Update 2 now would require someone to install it locally (can't be installed side-by-side with Update 3), make the necessary workarounds and then hope it doesn't break in the future as there is no CI support. Not really a viable way to go.

@wjakob
Copy link
Member

wjakob commented Mar 18, 2017

For me it's fine to draw the line at 2015u3. Part of the motivation of this project is that it's okay to assume a reasonably correct compiler to avoid having to build complicated abstractions over semi-broken metaprogramming facilities.

@jagerman
Copy link
Member

I went through the list of PRs merged since 2.0, and tagged the ones that I think belong in the changelog with the v2.1 tag. I left out of a bunch of the minor fixes, and PRs for fixes to things merged since 2.1. I might have also missed a few, but it's a starting point.

@jagerman
Copy link
Member

#746 is pretty fresh, but it's non-core and seems to work; I think we should consider including it in 2.1 (assuming there's no big objections once interested parties respond), but also don't mind deferring it to 2.2 if it's "too new."

@wjakob
Copy link
Member

wjakob commented Mar 19, 2017

It's only adding features rather than changing existing functionality. The feature is very useful in this case. I'd vote to include it in 2.1.

@wjakob
Copy link
Member

wjakob commented Mar 22, 2017

v2.1.0 is now released. Thank you everyone! 🎉

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

No branches or pull requests

4 participants