Work done while building for 2.10, which is not specific to 2.10 #1392

Merged
merged 7 commits into from Jan 7, 2013

Conversation

Projects
None yet
2 participants
@nafg
Contributor

nafg commented Jan 6, 2013

No description provided.

@nafg

This comment has been minimized.

Show comment Hide comment
@nafg

nafg Jan 7, 2013

Contributor

Hmm come to think of it, it's silly to make Props.mode non-lazy when Props itself is a lazy object, thus anyway necessitating touching it in a place like MailerSpec which forks a thread in a test. I may as well leave it a lazy val and touch them together.
Let me push -f.

Contributor

nafg commented Jan 7, 2013

Hmm come to think of it, it's silly to make Props.mode non-lazy when Props itself is a lazy object, thus anyway necessitating touching it in a place like MailerSpec which forks a thread in a test. I may as well leave it a lazy val and touch them together.
Let me push -f.

@nafg

This comment has been minimized.

Show comment Hide comment
@nafg

nafg Jan 7, 2013

Contributor

Okay updated. Please review.

Contributor

nafg commented Jan 7, 2013

Okay updated. Please review.

@Shadowfiend

This comment has been minimized.

Show comment Hide comment
@Shadowfiend

Shadowfiend Jan 7, 2013

Owner

Fwiw, the Box check used to work because:

  override def equals(other: Any): Boolean = (this, other) match {
    case (Full(x), Full(y)) => x == y
    case (Full(x), y) => x == y
    case (x, y: AnyRef) => x eq y
    case _ => false
  }

Full(C) == C is always true if C == C.

Owner

Shadowfiend commented Jan 7, 2013

Fwiw, the Box check used to work because:

  override def equals(other: Any): Boolean = (this, other) match {
    case (Full(x), Full(y)) => x == y
    case (Full(x), y) => x == y
    case (x, y: AnyRef) => x eq y
    case _ => false
  }

Full(C) == C is always true if C == C.

@@ -40,12 +40,14 @@ class JsonBoxSerializer extends Serializer[Box[_]] {
extract(chain, TypeInfo(BoxClass, Some(typeHoldingFailure))).asInstanceOf[Box[Failure]])
case JObject(JField("box_failure", JString("ParamFailure")) ::
JField("msg", JString(msg)) ::
- JField("exception", exception) ::
+ JField("exception", exn) ::

This comment has been minimized.

Show comment Hide comment
@Shadowfiend

Shadowfiend Jan 7, 2013

Owner

Was there a reason to shorten this variable name?

@Shadowfiend

Shadowfiend Jan 7, 2013

Owner

Was there a reason to shorten this variable name?

This comment has been minimized.

Show comment Hide comment
@nafg

nafg Jan 7, 2013

Contributor

This whole file is Joni's patch, which he sent on the mailing list.
Note that previously it wasn't even used.

@nafg

nafg Jan 7, 2013

Contributor

This whole file is Joni's patch, which he sent on the mailing list.
Note that previously it wasn't even used.

This comment has been minimized.

Show comment Hide comment
@Shadowfiend

Shadowfiend Jan 7, 2013

Owner

That's fine. I just see no reason to shorten the name. It reduces clarity. Do you mind switching it back?

@Shadowfiend

Shadowfiend Jan 7, 2013

Owner

That's fine. I just see no reason to shorten the name. It reduces clarity. Do you mind switching it back?

@nafg

This comment has been minimized.

Show comment Hide comment
@nafg

nafg Jan 7, 2013

Contributor

Ouch! Does Box#== still do that? Why?

Contributor

nafg commented Jan 7, 2013

Ouch! Does Box#== still do that? Why?

@nafg

This comment has been minimized.

Show comment Hide comment
@nafg

nafg Jan 7, 2013

Contributor

@Shadowfiend Thanks for reviewing! Can I merge it?

Contributor

nafg commented Jan 7, 2013

@Shadowfiend Thanks for reviewing! Can I merge it?

@Shadowfiend

This comment has been minimized.

Show comment Hide comment
@Shadowfiend

Shadowfiend Jan 7, 2013

Owner

Box#== does still do that, yes. Presumably as a convenience. Not sure I disagree, though we should make sure that Box#hashCode for a Full also passes through to its contained object if we're going about it that way.

Although now that I look at it, there's a Box#=== that is meant to do exactly that (compare to the contents of a Full or return false otherwise). So maybe Box#== should be switched to the other thing.

Anyway, except for the variable name complaint, yeah, +1 from me.

Owner

Shadowfiend commented Jan 7, 2013

Box#== does still do that, yes. Presumably as a convenience. Not sure I disagree, though we should make sure that Box#hashCode for a Full also passes through to its contained object if we're going about it that way.

Although now that I look at it, there's a Box#=== that is meant to do exactly that (compare to the contents of a Full or return false otherwise). So maybe Box#== should be switched to the other thing.

Anyway, except for the variable name complaint, yeah, +1 from me.

@Shadowfiend

This comment has been minimized.

Show comment Hide comment
@Shadowfiend

Shadowfiend Jan 7, 2013

Owner

Maybe let's bring up Box#== on the list and see if there are any other thoughts.

Owner

Shadowfiend commented Jan 7, 2013

Maybe let's bring up Box#== on the list and see if there are any other thoughts.

@nafg

This comment has been minimized.

Show comment Hide comment
@nafg

nafg Jan 7, 2013

Contributor

@Shadowfiend Okay I changed the name. Sublime Text's multiple selection is nice. :)

Contributor

nafg commented Jan 7, 2013

@Shadowfiend Okay I changed the name. Sublime Text's multiple selection is nice. :)

@Shadowfiend

This comment has been minimized.

Show comment Hide comment
@Shadowfiend

Shadowfiend Jan 7, 2013

Owner

♥ Merge 'er in!

Owner

Shadowfiend commented Jan 7, 2013

♥ Merge 'er in!

@nafg nafg merged commit 9e2f87f into master Jan 7, 2013

@nafg

This comment has been minimized.

Show comment Hide comment
@nafg

nafg Jan 7, 2013

Contributor

Thanks :)

Contributor

nafg commented Jan 7, 2013

Thanks :)

@nafg nafg deleted the nafg_pre_210 branch Jan 7, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment