Created attachment 702065 [details] Difference between Node's deepEqual and nodejs-deep-equal's deepEqual The nodejs-deep-equal package is currently sitting in [updates-testing], but there are some questions to be asked about bundling. It is essentially a fork of Node's assert module. This is the description from the website: https://github.com/substack/node-deep-equal > Node's assert.deepEqual() algorithm as a standalone module. > This module is around 5 times faster than wrapping assert.deepEqual() in a try/catch. The difference between Node's deepEqual and this package's deepEqual is attached. There are a number of Node.js modules that prefer to use the forked nodejs-deep-equal due to the increase in speed. The package is not mine, but I reviewed/approved the package. I'm not really sure what the best course of action is at the moment.
I should note that similar questions are being asked about the nodejs-should package which is currently undergoing review: https://bugzilla.redhat.com/show_bug.cgi?id=911188
I should also note that the bundled library that nodejs-should contains is in fact yet another fork of the deepEqual function from Node's assert.js.
This is clearly a fork, the motivation behind which is mostly the fact that node/JS exceptions suck. The end result is that it's much faster than the standard library implementation. It also can be used in the browser, unlike the standard library implementation. These reasons for forking are laid out in the module's README: https://github.com/substack/node-deep-equal/blob/master/README.markdown AFAIK Fedora is okay with clear forks (c.f. MATE, LibreOffice, etc.). The No Bundled Libraries page speaks of forks bundled with other packages, but not forked libraries in their own right. Note that should is a different beast since it bundles a library instead of providing a library.
Great. I wasn't sure how to interpret this situation. My opinion was pretty much the same as yours, but I wanted to be sure that this was fine for Fedora.
Yes the No Bundled Libraries talks of forking - as a reason why bundled libraries are bad! I don't see anything which clearly allows this - what is not clear to me is when copy and pasting a file or function becomes bundling. It's not bundling it the traditional sense of putting a copy of the whole upstream tar ball in a subdirectory obviously. The text in the README explains why, but the No Bundled Libraries response to that would be that upstream should be encouraged to make the underlying _deepEqual public for users that want to avoid the exception.
(Note: I'm not going to address the copy and pasting question here because it's a grey area that currently, the FPC decides on a case-by-case basis). We (FPC) like to avoid forks. However, when a new library is created that forks some other project in order to improve on the functionality because the originating upstream is not willing to do that, we still consider that a win overall. By making the improvements into a library that multiple projects can use the hope is to cut down on number of bundled instances and consolidate other projects on a single implementation (the new library that contains the improvements). We do most often encounter this situation when we're talking to a maintainer or upstream about a case of bundling and convince them to instead release their forked library separately for all to use and improve as a compromise but the idea should hold for cases where this is proactively done as well. If we did have any questions they would probably be: * Did the forking library try to get their changes incorporated into the original library? What was the result? * How open are the developers to new changes from other sources? The idea is to promote consolidation of changes into one codebase. I don't think the FPC has this officially recorded somewhere. It would probably be good for us to put into the Guidelines explicitly. (Especially because I'm just one member.. until the FPC votes on it, this is just my opinion of how the FPC would rule on it, not what the FPC actually will do :-)
(In reply to comment #6) > If we did have any questions they would probably be: > * Did the forking library try to get their changes incorporated into the > original library? What was the result? Not sure, though I can say for certain that they would *not* be accepted upstream. lib/assert.js in node core is designed to be a dead simple, throw an exception and stop execution assert function similar to those provided by virtually every programming language known to man. Doing anything fancier is the domain of test frameworks, which this was designed to be used with. > * How open are the developers to new changes from other sources? The idea > is to promote consolidation of changes into one codebase. substack is generally good about accepting sensible pull requests and accepted the only one ever filed against this particular library: https://github.com/substack/node-deep-equal/pulls?direction=desc&page=1&sort=created&state=closed I'll comment on the broader issues in the more appropriate venues it's already being discussed in.
(In reply to comment #7) > Not sure, though I can say for certain that they would *not* be accepted > upstream. Just to back this up with something besides my opinion, upstream lists this module as "locked": http://nodejs.org/api/assert.html So they most certainly will not be changing the semantics of this library.
Well currently upstream has deepEqual that basically looks like: assert(_deepEqual(...)) and the internal _deepEqual routine is the one people keep pulling out in order to get the comparison semantics without the assertion. So upstream could rename _deepEqual as deepEqualNoAssert (or whatever) and make it available without changing the semantics of the current public routines. Or they could move it to a separate comparison module that was then used by the assert module to implement assert.deepEqual.
Unfortunately that probably will not happen. http://nodejs.org/api/documentation.html: > Stability: 5 - Locked > Unless serious bugs are found, this code will not ever > change. Please do not suggest changes in this area; they will be refused. You're welcome to go against their wishes and file a bug though. ;-)