Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

tools: add support for import assertions in linter #39924

Closed
wants to merge 2 commits into from
Closed

tools: add support for import assertions in linter #39924

wants to merge 2 commits into from

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Aug 28, 2021

Necessary for #39921.

@nodejs-github-bot nodejs-github-bot added the tools Issues and PRs related to the tools directory. label Aug 28, 2021
@aduh95 aduh95 mentioned this pull request Aug 28, 2021
Closed
@aduh95 aduh95 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 28, 2021
Trott
Trott approved these changes Aug 31, 2021
View reviewed changes
Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rubber stamp LGTM

@jasnell
Copy link
Member

jasnell commented Sep 3, 2021

Landed in f26c2ce

@jasnell jasnell closed this Sep 3, 2021
jasnell pushed a commit that referenced this pull request Sep 3, 2021
@aduh95 @jasnell
tools: add support for import assertions in linter
f26c2ce 
PR-URL: #39924
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
@aduh95 aduh95 deleted the import-assertions-linter branch September 3, 2021 14:51
targos pushed a commit that referenced this pull request Sep 4, 2021
@aduh95 @targos
tools: add support for import assertions in linter
7b2bebb 
PR-URL: #39924
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
@targos targos mentioned this pull request Sep 4, 2021
Merged
targos pushed a commit that referenced this pull request Sep 6, 2021
@aduh95 @targos
tools: add support for import assertions in linter
a0971a2 
PR-URL: #39924
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
targos pushed a commit that referenced this pull request Sep 6, 2021
@aduh95 @targos
tools: add support for import assertions in linter
d92bd9a 
PR-URL: #39924
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
@targos targos mentioned this pull request Sep 6, 2021
Merged
@aduh95 aduh95 restored the import-assertions-linter branch October 14, 2021 15:29
@aduh95
tools: add support for import assertions in linter
2cbfc9f 
PR-URL: nodejs#39924
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
@aduh95
Copy link
Contributor Author

aduh95 commented Oct 14, 2021

This was (erroneously I think) removed in #40394, re-opening.

@aduh95 aduh95 reopened this Oct 14, 2021
@aduh95 aduh95 force-pushed the import-assertions-linter branch from f6e527b to 2cbfc9f Compare October 14, 2021 15:34
@targos
Copy link
Member

targos commented Oct 14, 2021

It seems like you're right, but if nothing breaks when we remove it, doesn't it prove that we don't need it (yet) ?

@aduh95
Copy link
Contributor Author

aduh95 commented Oct 14, 2021

It seems like you're right, but if nothing breaks when we remove it, doesn't it prove that we don't need it (yet) ?

Fair, added a test.

@aduh95 aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 14, 2021
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 14, 2021
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/40306/

GeoffreyBooth
GeoffreyBooth approved these changes Oct 14, 2021
View reviewed changes
Copy link
Member

@GeoffreyBooth GeoffreyBooth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need this in order to land #40250.

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/40308/

@github-actions github-actions bot mentioned this pull request Oct 15, 2021
Open
23 tasks
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/40310/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/40311/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/40315/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/40316/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/40319/

This was referenced Oct 16, 2021
Open
Open
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/40333/

@aduh95 aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 17, 2021
@github-actions github-actions bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 17, 2021
@github-actions
Copy link
Contributor

Landed in cbc7b5d...f5e9486

@github-actions github-actions bot closed this Oct 17, 2021
nodejs-github-bot pushed a commit that referenced this pull request Oct 17, 2021
@aduh95 @nodejs-github-bot
tools: add support for import assertions in linter
f5e9486 
PR-URL: #39924
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>

Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
@aduh95 aduh95 deleted the import-assertions-linter branch October 17, 2021 18:56
This was referenced Oct 18, 2021
Open
Open
Open
Open
Open
Open
targos pushed a commit that referenced this pull request Nov 4, 2021
@aduh95 @targos
tools: add support for import assertions in linter
abf3b84 
PR-URL: #39924
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>

Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
@BethGriggs BethGriggs mentioned this pull request Nov 26, 2021
Merged
1 task
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@bmeck bmeck bmeck approved these changes

@jasnell jasnell jasnell approved these changes

@GeoffreyBooth GeoffreyBooth GeoffreyBooth approved these changes

@Trott Trott Trott approved these changes

@watilde watilde watilde approved these changes

@targos targos targos approved these changes

@Lxxyx Lxxyx Lxxyx approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. tools Issues and PRs related to the tools directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@aduh95 @jasnell @targos @nodejs-github-bot @bmeck @GeoffreyBooth @Trott @watilde @Lxxyx