Backporting naming anonymous functions to v6 / v4

[MylesBorins]

nodejs/node#8913 started the conversation about naming anonymous functions, and we have had a slew of contributions around this

PR's include (incomplete list):

• http: name anonymous functions node#9054
• http: name anonymous functions node#9055
• assert: name anonymous functions node#9051

Should we be backporting these?
Have these all ended up doing what we expected and indeed improved the situation? I know there was some concern around whether or not some of the things being named were actually improving the situation. Has anything landed that shouldn't?

I think auditing all of these PR's in one go will be a good to to ensure that we have indeed improved the situation. That being said, should it be backported. I'm leaning to yes

/cc @nodejs/ctc @Raynos

[sam-github]

If they don't get backported, because of the code churn, future PRs will get increasingly harder to back port, won't they?

I think some of the refactors might have gotten a thumbs up too quickly, I saw some weird code in cluster, and some partial es6ization of functions, creating a random mixture of es6 and function, so not quite achieving consistency.

[brodycj]

👎 for backporting on my part. For new features and improvements people should just upgrade. I can think of the possible approach for critical and security bug fixes:

• fix in older version(s), use git tool to help port the fix to newer version(s), and rework if necessary
• fix in newer version(s), backport each critical/security fix to the older version(s), and rework if necessary

[sam-github]

@brodybits I believe node policy is to backport semver-minor features, not just bug/security fixes.

[brodycj]

From README.md:

Changes in an LTS-covered major version are limited to:

1. Bug fixes;
2. Security updates;
3. Non-semver-major npm updates;
   ...

Does this imply that every "non-semver-major" improvement may have to be backported to all active LTS releases at some point? If so, is there some criteria when every "non-semver-major" improvement should be backported? If not is there any criteria to define which improvements have to be backported and when they should be backported?

[MylesBorins]

@brodybits that 3rd point is specifically about npm updates.

Points 4 and 5 specify the important bits you are looking for

1. Certain performance improvements where the risk of breaking existing applications is minimal;
2. Changes that introduce large amount of code churn where the risk of breaking existing applications is low and where the change in question may significantly ease the ability to backport future changes due to the reduction in diff noise.

We do backport performance improvements that are deemed to have little chance of breaking things. We also backport changes with large code churn that have low risk of breaking things.

These changes fall into the category of point 5

[brodycj]

Thanks!

[jasnell]

I'm ok with backporting these but I would prefer to wait a bit to ensure that there are no significant regressions reported. I do not expect there to be any but there is a non-zero percent chance that this could impact someone (even if that chance is really really close to zero).

[MylesBorins]

sgtm. I really think we should be doing a batch review of all these changes anyways. My gut tells me we may have ended up regressing in the stack traces in some places based on some of the comments I've seen

[thefourtheye]

I am not sure how to verify if the changes we introduced fix the original issue raised by @Raynos. Can someone help me with that?

[MylesBorins]

I think we are fine with where we are so far. If people want this they would ask for it.