[v20.x backport] backport: esm: require braces for modules code#50268
Closed
GeoffreyBooth wants to merge 1 commit intonodejs:v20.x-stagingfrom
Closed
[v20.x backport] backport: esm: require braces for modules code#50268GeoffreyBooth wants to merge 1 commit intonodejs:v20.x-stagingfrom
GeoffreyBooth wants to merge 1 commit intonodejs:v20.x-stagingfrom
Conversation
Collaborator
|
Review requested:
|
nodejs-github-bot
added
lib / src
PR-URL: nodejs#49657 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me>
GeoffreyBooth
force-pushed
the
backport-49657-to-20
branch
from
fc934ad to
83e582a
Compare
GeoffreyBooth
changed the title
GeoffreyBooth
removed
the
lib / src
Collaborator
GeoffreyBooth
changed the title
targos
approved these changes
Oct 19, 2023
JakobJingleheimer
approved these changes
Oct 19, 2023
aduh95
reviewed
Oct 19, 2023
Comment on lines
+114
to
+121
| { | ||
| files: [ | ||
| 'lib/internal/modules/**/*.js', | ||
| ], | ||
| rules: { | ||
| 'curly': 'error', | ||
| }, | ||
| }, |
Contributor
There was a problem hiding this comment.
Can you remove this to avoid getting error when backporting commits that landed before this rule was introduced?
Member
There was a problem hiding this comment.
If there are commits to backport from before this rule, I highly recommend to backport them first.
Contributor
There was a problem hiding this comment.
I don't know if they actually are any, regardless having less strict lint rules on LTS lines seems like a good idea to me, but it's just a nit
Member
Author
There was a problem hiding this comment.
So I looked into this, and the only two commits from before this rule are:
- src: throw DataCloneError on transfering untransferable objects #47604, which is semver-major so we won’t backport it.
- esm: remove
globalPreloadhook (superseded byinitialize) #49144, which @aduh95 requested not be backported. It also wouldn’t run afoul of this rule regardless, because it’s almost all deletions and the few additions include the newly required braces.
So I think the rule is fine to backport, and I’d rather not create unnecessary divergences from main. This rule is also very easy to abide by, and the linter can fix for it automatically.
This was referenced Oct 20, 2023
Member
|
Landed in 0103979 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This backports:
Resolves #49657 (comment).