Implementation and design aspects of the Scala system internals ()

Josh Suereth | 4 Aug 2012 16:45

Patch review backup

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

headers

Eugene Burmako | 4 Aug 2012 16:55

Re: Patch review backup

Umm wait! Martin said we're going to *just discuss* the possibility of the release on the next Scala meeting (Tuesday).

On 4 August 2012 16:45, Josh Suereth <joshua.suereth <at> gmail.com> wrote:

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

headers

Adriaan Moors | 4 Aug 2012 16:58

Re: Patch review backup

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 <at> noon.

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

On Sat, Aug 4, 2012 at 4:45 PM, Josh Suereth <joshua.suereth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

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

headers

Josh Suereth | 4 Aug 2012 17:14

Re: Patch review backup

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.

On Aug 4, 2012 10:58 AM, "Adriaan Moors" <adriaan.moors-p8DiymsW2f8@public.gmane.org> wrote:

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 <at> noon.

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

On Sat, Aug 4, 2012 at 4:45 PM, Josh Suereth <joshua.suereth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
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

headers

Adriaan Moors | 4 Aug 2012 17:43

Re: Patch review backup

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.

On Sat, Aug 4, 2012 at 5:14 PM, Josh Suereth <joshua.suereth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

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.

On Aug 4, 2012 10:58 AM, "Adriaan Moors" <adriaan.moors-p8DiymsW2f8@public.gmane.org> wrote:

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 <at> noon.

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

On Sat, Aug 4, 2012 at 4:45 PM, Josh Suereth <joshua.suereth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
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

headers

Paul Phillips | 4 Aug 2012 17:46

Re: Patch review backup

On Sat, Aug 4, 2012 at 8:43 AM, Adriaan Moors <adriaan.moors-p8DiymsW2f8@public.gmane.org> 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.

headers

Adriaan Moors | 4 Aug 2012 17:51

Re: Patch review backup

I never said anything divisive, did I?

On Sat, Aug 4, 2012 at 5:46 PM, Paul Phillips <paulp-v5eHc9rg9U0h9ZMKESR00Q@public.gmane.org> wrote:

On Sat, Aug 4, 2012 at 8:43 AM, Adriaan Moors <adriaan.moors <at> 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.

headers

Vlad Ureche | 4 Aug 2012 22:03

Re: Patch review backup

On Sat, Aug 4, 2012 at 5:14 PM, Josh Suereth <joshua.suereth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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

headers

Paul Phillips | 4 Aug 2012 23:21

Re: Patch review backup

On Sat, Aug 4, 2012 at 1:03 PM, Vlad Ureche <vlad.ureche-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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 <at> specialize, interactions accompany every change, and I doubt tomorrow will be the day that someone arrives to make specialization their life's mission.