Skip to content

Fix ConsoleInterface binding things properly^2#386

Merged
eed3si9n merged 1 commit intosbt:1.0.xfrom
dwijnand:consoleProject
Sep 8, 2017
Merged

Fix ConsoleInterface binding things properly^2#386
eed3si9n merged 1 commit intosbt:1.0.xfrom
dwijnand:consoleProject

Conversation

@dwijnand
Copy link
Member

Follow-up on #314 - I still misinterpreted..

Turns out the ".asInstanceOf[AnyRef].getClass.getName" implementation
was the original implementation. Then Mark switched to using bindValue
in sbt/sbt@4b8f0f3.

Since Scala 2.11.0 (scala/scala#1648 in particular) bindValue was
removed. So we'll use NamedParam and quietBind, both which exist since
Scala 2.9.0.

Fixes sbt/sbt#2884, tested with local releases.

@dwijnand dwijnand added this to the 1.0.1 milestone Jul 31, 2017
jvican
jvican previously approved these changes Jul 31, 2017
Copy link
Member

@jvican jvican left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@jvican
Copy link
Member

jvican commented Aug 1, 2017

Why is this targetting 1.0.1?

@dwijnand
Copy link
Member Author

dwijnand commented Aug 1, 2017

Because @eed3si9n targeted the issue to 1.0.1: sbt/sbt#2884 (comment)

@eed3si9n
Copy link
Member

eed3si9n commented Aug 4, 2017

If it's not going to be part of 1.0.0, then I don't think we should merge it in there.
Should we create a branch for 1.0.x?

@dwijnand
Copy link
Member Author

dwijnand commented Aug 4, 2017

I thought about that, but I'd rather not get into the business of more persistent branches.

I think we should merge it into the 1.0.0 branch and here are my reasons:

  1. I think the bug fix here is clean and simple, and doesn't impact anything outside of console.

  2. If we do discover some release-blocking regression that forces our hand to do an RC4 release, this bug fix can go with it, imo. If we don't then this fix can ship as 1.0.1 or 1.1.0 at a later date.

@dwijnand
Copy link
Member Author

dwijnand commented Aug 4, 2017

Oh and at this point in time the only thing in the 1.x branch that's not in the 1.0.0 branch is this inconsequential test classpath change: #328.

@jvican jvican modified the milestones: 1.0.0, 1.0.1 Aug 4, 2017
@jvican
Copy link
Member

jvican commented Aug 4, 2017

Let's then aim for 1.0.0. I'm fine merging it and releasing RC4.

@eed3si9n
Copy link
Member

eed3si9n commented Aug 4, 2017

@dwijnand

I think we should merge it into the 1.0.0 branch and here are my reasons ...

So you are saying that RC-3 is still on its way to become 1.0.0 final, but you still want to merge this into 1.0.0 release branch? 1.0.0 is effectively a mutable tag that we are saying it will become 1.0.0 eventually.

If we plan to release 1.0.1 with bug fixes only, then we should create 1.0.x. Otherwise, we should merge this to 1.x and ship in 1.1.0 (in a few months?).

@jvican

Let's then aim for 1.0.0. I'm fine merging it and releasing RC4.

Releasing RC-4 means delaying the entire sbt 1.0 and Zinc 1.0 for another week or two.
I personally don't think this is worth it.

@jvican
Copy link
Member

jvican commented Aug 4, 2017

Releasing RC-4 means delaying the entire sbt 1.0 and Zinc 1.0 for another week or two.
I personally don't think this is worth it.

Hmmm... if releasing a trivial fix is going to delay sbt 1.0, then we have a real problem with our tooling. Personally, I don't like making wrong decisions because of constraints on the tools I use -- if the current repo setup is causing too much pain, we should have a monorepo or something that emulates it.

@eed3si9n
Copy link
Member

eed3si9n commented Aug 5, 2017

Hmmm... if releasing a trivial fix is going to delay sbt 1.0, then we have a real problem with our tooling.

It's a matter of policy, not tooling. Even if we had a monorepo e.g. sbt/sbt, I would still push the final date by a week or two if we made any change to the code before calling it 1.0.0.

@dwijnand dwijnand changed the base branch from 1.0.0 to 1.x August 14, 2017 15:53
Follow-up on sbt#314 - I _still_ misinterpreted..

Turns out the ".asInstanceOf[AnyRef].getClass.getName" implementation
was the _original_ implementation. Then Mark switched to using bindValue
in sbt/sbt@4b8f0f3.

Since Scala 2.11.0 (scala/scala#1648 in particular) bindValue was
removed. So we'll use NamedParam and quietBind, both which exist since
Scala 2.9.0.

Fixes sbt/sbt#2884, tested with local releases.
@dwijnand dwijnand modified the milestones: 1.0.1, 1.0.0 Sep 6, 2017
@dwijnand dwijnand changed the base branch from 1.x to 1.0.x September 8, 2017 16:25
@eed3si9n eed3si9n merged commit 25ad019 into sbt:1.0.x Sep 8, 2017
@dwijnand dwijnand deleted the consoleProject branch July 16, 2020 14:01
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.

3 participants