Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

tools: bump remark-preset-lint-node to 3.0.0 #39755

Closed
wants to merge 1 commit into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Aug 13, 2021

No description provided.

@nodejs-github-bot nodejs-github-bot added the tools Issues and PRs related to the tools directory. label Aug 13, 2021
@targos
Copy link
Member

targos commented Aug 13, 2021

Would it be possible to dedupe the module tree a little bit (I usually delete node_modules and the package-lock.json file before running npm install to get the best results)?

@Trott
Copy link
Member Author

Trott commented Aug 13, 2021

Would it be possible to dedupe the module tree a little bit (I usually delete node_modules and the package-lock.json file before running npm install to get the best results)?

Unfortunately, no, not without additional changes:

$ make lint-md
Running JS linter...
Running Markdown linter...
/Users/trott/io.js/tools/lint-md.js:12014
glob$1.Glob;
       ^

TypeError: Cannot read property 'Glob' of undefined
    at Object.<anonymous> (/Users/trott/io.js/tools/lint-md.js:12014:8)
    at Module._compile (node:internal/modules/cjs/loader:1101:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:79:12)
    at node:internal/main/run_main_module:17:47
make: *** [tools/.mdlintstamp] Error 1
$  

I think this is due to incompatibilities with/between a new rollup and an old @rollup/plugin-commonjs that doesn't work with our code. The next thing I was going to do after this lands is ESM-ify our local code, and then I think updating rollup will work.

Let me make those change now and see if it fixes it. If it does, I'll add that commit to this PR.

@Trott
Copy link
Member Author

Trott commented Aug 13, 2021

Let me make those change now and see if it fixes it. If it does, I'll add that commit to this PR.

No luck, but I did get it to work without rollup. I think it's time to de-rollup-ify this tool, which I will do right after this lands. (I've got it queued up, but there are some license things to go over and possible other issues that I don't want to hold this PR up with.)

@targos
Copy link
Member

targos commented Aug 13, 2021

My comment is non-blocking.

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

Landed in fcf8ba4...75bd4da

@github-actions github-actions bot closed this Aug 15, 2021
nodejs-github-bot pushed a commit that referenced this pull request Aug 15, 2021
PR-URL: #39755
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
danielleadams pushed a commit that referenced this pull request Aug 16, 2021
PR-URL: #39755
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
targos pushed a commit that referenced this pull request Sep 4, 2021
PR-URL: #39755
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
@Trott Trott deleted the preset@3 branch September 25, 2022 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants