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.

[nlf]

The existing pattern of documenting breaking changes for any hapi related project has always been the milestones. As Eran said, updating without looking to see what changed is what caused the issue here.

However, I would agree that you not looking was likely caused by the fact that finding the milestones is a pain in the ass. I have a couple of ideas to help make this easier for everyone in the future.

As for your comment, @enjikaka, your hostility is not welcome here. We all work on these projects for free in our spare time after we work another full time job. If something isn't right, I know it's frustrating but suggesting ways to resolve it is 10,000,000 times more helpful than telling a maintainer what they can and cannot do. It's open source. If you don't like something, fix it.

[devinivy]

This is free software open for improvement by everyone.

In this case the docs were updated, but a simple mistake was made in updating the doc's heading to reflect the new version. The hapijs organization reliably uses milestones to track changes, and in the future if there's any question as to what has changed with a version bump, you can at the very least check there. If you find that a mistake was made, usually a friendly and constructive issue or a PR will do the trick to fix it.

[enjikaka]

@nlf Not trying to be hostile. If you've ever use other peoples code enough I'm sure you've encountered the frustation when developers do not keep their documentation up to date so things just don't work out for others, and you have to spend a week trying to figure out why. What seems to be a small thing to miss out on can actually have a really big and bad effect for many people. This is the worst kind of bad DX.
I see that it was documented, but not easily accessible. That's important too. I'm not a user of this, but for all that do use it; please keep this in mind next time. Good luck in the future.

[lock]

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.