Patch review backup

68 views
Skip to first unread message

Josh Suereth

unread,
Aug 4, 2012, 10:45:20 AM8/4/12
to scala-i...@googlegroups.com
Hey Scala committers!

We have a pull request backup from reviews/extra work.   Since we want to release monday, I need to see an unambiguous LGTM or rework to allow these patches.    Monday COB (5pm EDT) I'm going to close patches that have no clear LGTM and have been hanging in the hopper for long enough, I assume they are stale.

I'd like to ensure we get *all* critical fixes into 2.10.x branch before we can cut a 2.10.0-RC1.   I'll be running community builds and such, but if your patch doesn't get reviewed and in, it could hold up the release.

That's right, failure to appropriately review a fix can hold up the release just as easily as unfixed critical bug.   Let's not let that happen!   I know there's going to be a rush this week to push a lot of code, let's also push to review code as well.

- Josh

Eugene Burmako

unread,
Aug 4, 2012, 10:55:25 AM8/4/12
to scala-i...@googlegroups.com
Umm wait! Martin said we're going to *just discuss* the possibility of the release on the next Scala meeting (Tuesday).

Adriaan Moors

unread,
Aug 4, 2012, 10:58:22 AM8/4/12
to scala-i...@googlegroups.com
I'm not sure I'm ready for RC1 due to the regressions caused by my fix in https://github.com/scala/scala/pull/937
I hope to have this and the other remaining issues fixed by Tuesday evening/Wednesday@noon.

[I just got back from vacation and haven't caught up with mail backlog, sorry]

Josh Suereth

unread,
Aug 4, 2012, 11:14:25 AM8/4/12
to scala-i...@googlegroups.com

No matter what happens the next two weeks or so, a backup of 20 patches is not a good thing.   I can keep on top of merging I'd everyone can keep on top of reviewing.

We cant discus a release if patches are not even reviewed/merged.

I do agree that Tues. Seems far too early for a release but let's be able to discuss what we have to do without baggage.

Adriaan Moors

unread,
Aug 4, 2012, 11:43:33 AM8/4/12
to scala-i...@googlegroups.com
I absolutely agree about the importance of reviews and keeping the pull request queue short.
My comment was supposed to be additive to your message, not subtractive. 

Paul Phillips

unread,
Aug 4, 2012, 11:46:32 AM8/4/12
to scala-i...@googlegroups.com


On Sat, Aug 4, 2012 at 8:43 AM, Adriaan Moors <adriaa...@epfl.ch> wrote:
I absolutely agree about the importance of reviews and keeping the pull request queue short.
My comment was supposed to be additive to your message, not subtractive. 

I divided them and came up with NaN.  Please enclose instructions in the future.

Adriaan Moors

unread,
Aug 4, 2012, 11:51:31 AM8/4/12
to scala-i...@googlegroups.com
I never said anything divisive, did I?

Vlad Ureche

unread,
Aug 4, 2012, 4:03:12 PM8/4/12
to scala-i...@googlegroups.com

On Sat, Aug 4, 2012 at 5:14 PM, Josh Suereth <joshua....@gmail.com> wrote:
No matter what happens the next two weeks or so, a backup of 20 patches is not a good thing.

Speaking of reviews, I'd like to bring pull request 1025 to attention.
In short:
 -- the AnyRef specialization triggers a bug in mixin (SI-6103) that prevents java code from instantiating AnyRef-specialized code, including Function and Tuple (Si-6101)
 -- the solution we discussed during the scala meeting is to hide AnyRef specialization under a flag, as it is currently fragile (other bugs involving AnyRef specialization: SI-6043, SI-4790, SI-5976 and SI-5583)

I implemented two solutions, neither of which got PaulP's LGTM: pullreq 1025 and pullreq 999. The reason in both cases is that the changes are involved, but they need to be so. Either way, we need to decide what to do about it, and if really both variants are too intricate, let's at least remove the AnyRef specialization from Function and Tuple, so we don't end up releasing a java-incompatible version of the library.

WDYT, how should we proceed?

Thanks,
Vlad 


Paul Phillips

unread,
Aug 4, 2012, 5:21:35 PM8/4/12
to scala-i...@googlegroups.com


On Sat, Aug 4, 2012 at 1:03 PM, Vlad Ureche <vlad....@gmail.com> wrote:
Either way, we need to decide what to do about it, and if really both variants are too intricate, let's at least remove the AnyRef specialization from Function and Tuple, so we don't end up releasing a java-incompatible version of the library.

I'll go along with whatever people want, but my vote would be to remove AnyRef specialization in preference to the schemes I've seen.  They may make perfect sense and have the minimum possible complexity, but even the minimum might be too much.  We've struggled for years with the bugs in @specialize, interactions accompany every change, and I doubt tomorrow will be the day that someone arrives to make specialization their life's mission.

Reply all
Reply to author
Forward
0 new messages