Default to deep copy

[hueniverse]

Here's a fucking crazy idea - default comparisons to deep. Why are we all typing .deep over and over again when almost no one needs shallow, reference comparison. I would even go as far as come up with a new method for .to.be.reference() to make your intention clear.

I'm so fucking sick of having to fix my tests because of a missing .deep modifier.

[cjihrig]

I agree with this idea. I think it would be simplest to just add a shallow modifier though.

[hueniverse]

I would like both. shallow and reference(). If we keep deep this will only break the very few places where we want to get the same reference. And by break I mean false positive.

[tanepiper]

While this is an appreciated change because deep behavior should be default, this morning I ended up spending two hours trying to figure out why recent tests broke in our latest update, and then refactoring the code to fix it. This change was not made visible (nothing in the release notes or changelog - and the API docs still reference 2.3.0) and no deprecation warning - just a sudden change. Milestones are not that visible in terms of documentation.

[cjihrig]

My bad on the API doc. The docs were correct, with the exception of the version at the top. That is fixed now.

[tanepiper]

@cjihrig Thanks, appreciated (and all the work you've done on Code so far)

[hueniverse]

@tanepiper so you moved to a new major version and just assumed nothing broke?!

[tanepiper]

@hueniverse Never assumed nothing would break, but it was not documented clearly - usually major breaking changes are clearly marked in changelogs, and like I said the API docs had not been updated clearly to reflect the version change at it was still referencing v2.3.x which caused some initial confusion.

I had to find out from twitter that it's here in the Milestones section of Code that it's documented that this would be happening, it's not visible from just the issues tab.

If you look at Git, milestones are not exactly the most discoverable way of locating this information:

Maybe editing in the release notes, which is easily accessible, to indicate it would have been good. Even a one liner like https://github.com/hapijs/code/releases/tag/v2.2.1

But as I also mentioned this is a welcome and sensible change, so I do appreciate the work done on it.

[hueniverse]

I am not arguing the change was clearly documented, just that I would expect you to first figure out what changed before actually making the transition...

[enjikaka]

"I would expect you to first figure out what changed"

That's going to be hard if you don't alert the users via release notes or docs. You should never EVER release without updating that, since it causes huge trouble for developers using your code then.