Hi Philip, Philip McGrath writes: > * gnu/packages/node.scm (node)[arguments]: Split 'patch-files phase > into 'patch-hardcoded-program-references and > 'delete-problematic-tests. Adapt those phases and 'configure to work > unmodified on node-lts. > (node, node-lts)[inputs]: Use bash-minimal rather than bash. > (node-lts)[arguments]: Inherit 'patch-hardcoded-program-references, > 'delete-problemating-tests, and 'configure phases from the bootstrap > node. Remove the 'patch-files phase, keeping its remaining > non-inherited work in a new 'replace-llhttp-sources phase. While I agree that most of the time, factoring out common code is a good thing, I'm not sure it applies in the case of patching tests. The list of tests is specific to a version and it's likely for each version to need fixes. Having a common phase that describes the tests to patch for 2 versions (3 if we add node 16) is harder to maintain than three phases IMO, even though they'll look similar indeed. Having to change commmon code can also cause unnessecary rebuilds. For example, I started working updating node last weekend and saw these test changes: - 14.16 -> 14.17: Delete test/parallel/test-https-agent-unref-socket.js, requires networking - 16: Extra test needs /bin/sh patched test/parallel/test-stdin-from-file-spawn.js" A couple tests were renamed: test/parallel/test-cluster-master-error.js -> test/parallel/test-cluster-primary-error.js test/parallel/test-cluster-master-kill.js -> test/parallel/test-cluster-primary-kill.js That being said, I definetely agree we should have a separate phase for the replacement of the llhttp source, that's logically different from patching tests, and is unlikely to change version to version. Keeping the list of tests local to each packages allows to add node 16 while avoiding rebuilding the others, does this make sense? I could be wrong here of course :-). Thanks, Pierre