tools: rolled up lint-md by refack · Pull Request #20109 · nodejs/node · GitHub

replace remark-cli and remark-preset-lint-node with a "rolled-up" single file.

Should solve all build & Makefile issue
very size economic (17KB instead of 20MB ~1300 files)
includes all recipes for re rolling & updating

/CC @wooorm @nodejs/build
H/T: @rollup

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
documentation is changed or added
commit message follows commit guidelines

https://ci.nodejs.org/job/node-test-commit-lite/709/

Nit: missing line break at EOF.

Please could you update

node/tools/license-builder.sh

Line 88 in e86b319

    
           addlicense "remark-cli" "tools/remark-cli" "$(cat ${rootdir}/tools/remark-cli/LICENSE)"

?

This sounds like a great solution to the problem. If it works we should probably consider doing the same for eslint.

The lint-md-build makefile target is removed, can you add it back with a command that runs this?

Since our CI runs this for every lint, I've made make lint-md-build a no-op, and added make lint-md-rollup

Very nice work. Thank you for working on this!

This is marked as WIP @refack is there anything else to do to get this landed (besides open comments)?

refack is there anything else to do to get this landed (besides open comments)?

I was hoping for some feedback on how we should track the license, and document this. Also i'm not sure how to test the new script...

@refack I think writing a bad .md in the root directory then running make lint-md expecting a non-zero exit code and some output complaining about it should be enough. Can be part of the doctool test suite. If we don't want to always run it we can just tweak the makefile and/or vcbuild.bat, it should be fine as long as it's run in the linter CI.

Ping @refack

I guess it is a good idea to do this but I am going to close this due to no response. If anyone feels like taking a stab at this, that would be great.

@refack please reopen in case you want to continue working on this.

Why should this not trigger the rollup?

Also lint-md-rollup uses npm up, and there is some spesific workaround code in the rollup config that could break:
https://github.com/nodejs/node/blob/67b6082e119b85551e5dcf4243b09f08f50f42ce/tools/node-lint-md-cli-rollup/rollup.config.js#L28-L36

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

PR-URL: nodejs#20109 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

* remove unused `tools/remark-cli` * vcbuild tested with `vcbuild nobuild noprojgen lint-md-build lint-md` PR-URL: nodejs#20109 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

Landed in
d3442f2
2120dac
7fcc178
fe6c74c
e4dd213

PR-URL: #20109 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

* remove unused `tools/remark-cli` * vcbuild tested with `vcbuild nobuild noprojgen lint-md-build lint-md` PR-URL: #20109 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

- In release guide - In Travis config Refs: nodejs#20109

- In release guide - In Travis config Refs: #20109 PR-URL: #22991 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

refack added wip Issues and PRs that are still a work in progress. doc Issues and PRs related to the documentations. build Issues and PRs related to build files or the CI. tools Issues and PRs related to the tools directory. labels Apr 17, 2018

refack self-assigned this Apr 17, 2018

refack requested review from MylesBorins, Trott, joyeecheung, targos, vsemozhetbyt and watilde April 17, 2018 16:40

nodejs-github-bot added build Issues and PRs related to build files or the CI. meta Issues and PRs related to the general management of the project. tools Issues and PRs related to the tools directory. labels Apr 17, 2018

vsemozhetbyt reviewed Apr 17, 2018

View reviewed changes

richardlau reviewed Apr 17, 2018

View reviewed changes

joyeecheung reviewed Apr 19, 2018

View reviewed changes

jasnell approved these changes Apr 19, 2018

View reviewed changes

devsnek approved these changes Apr 19, 2018

View reviewed changes

This was referenced Apr 28, 2018

tools: don't create package-lock.json #17330

Closed

build: search missing dependencies on lint-md #19249

Closed

bzoz mentioned this pull request May 2, 2018

win, build: add documentation support to vcbuild #19663

Closed

2 tasks

refack added 5 commits September 11, 2018 14:12

tools: vendor in node-lint-md-cli-rollup

d3442f2

PR-URL: nodejs#20109 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

tools: update node-lint-md-cli-rollup

2120dac

PR-URL: nodejs#20109 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

tools: relocate remark-preset-lint-node

7fcc178

PR-URL: nodejs#20109 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

tools: make lint-md-rollup & checkin lint-md.js

fe6c74c

PR-URL: nodejs#20109 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

tools: use lint-md.js

e4dd213

* remove unused `tools/remark-cli` * vcbuild tested with `vcbuild nobuild noprojgen lint-md-build lint-md` PR-URL: nodejs#20109 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

refack force-pushed the rolled-up-lint-md branch from 67b6082 to e4dd213 Compare September 11, 2018 18:13

refack merged commit e4dd213 into nodejs:master Sep 11, 2018

refack deleted the rolled-up-lint-md branch September 11, 2018 18:14

targos pushed a commit that referenced this pull request Sep 11, 2018

tools: vendor in node-lint-md-cli-rollup

37b27a5

PR-URL: #20109 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

targos pushed a commit that referenced this pull request Sep 11, 2018

tools: update node-lint-md-cli-rollup

25e1060

PR-URL: #20109 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

targos pushed a commit that referenced this pull request Sep 11, 2018

tools: relocate remark-preset-lint-node

350a3e1

PR-URL: #20109 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

targos pushed a commit that referenced this pull request Sep 11, 2018

tools: make lint-md-rollup & checkin lint-md.js

22fefae

PR-URL: #20109 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

targos mentioned this pull request Sep 18, 2018

Release proposal: v10.11.0 #22932

Merged

targos added a commit to targos/node that referenced this pull request Sep 21, 2018

build,doc: remove outdated lint-md-build

07cd754

- In release guide - In Travis config Refs: nodejs#20109

targos mentioned this pull request Sep 21, 2018

build,doc: remove outdated lint-md-build #22991

Closed

refack removed their assignment Oct 12, 2018

richardlau mentioned this pull request Nov 5, 2018

possible make lint-md-build bug #21648

Closed

tools: rolled up lint-md by refack · Pull Request #20109 · nodejs/node · GitHub

Uh oh!

Uh oh!

tools: rolled up lint-md - #20109

tools: rolled up lint-md#20109refack merged 5 commits intonodejs:masternodejs/node:masterfrom refack:rolled-up-lint-mdrefack/node:rolled-up-lint-mdCopy head branch name to clipboard

Conversation

refack commented Apr 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

refack commented Apr 17, 2018

Uh oh!

vsemozhetbyt Apr 17, 2018

Choose a reason for hiding this comment

Uh oh!

richardlau Apr 17, 2018

Choose a reason for hiding this comment

Uh oh!

gibfahn commented Apr 18, 2018

Uh oh!

joyeecheung Apr 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

refack Sep 6, 2018

Choose a reason for hiding this comment

Uh oh!

joyeecheung commented Apr 19, 2018

Uh oh!

BridgeAR commented Apr 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

refack commented Apr 27, 2018

Uh oh!

joyeecheung commented Apr 28, 2018

Uh oh!

BridgeAR commented May 15, 2018

Uh oh!

BridgeAR commented May 25, 2018

Uh oh!

refack commented Sep 6, 2018

Uh oh!

refack commented Sep 7, 2018

Uh oh!

refack commented Sep 11, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

tools: rolled up lint-md#20109
refack merged 5 commits into
nodejs:masterfrom
refack:rolled-up-lint-md

refack commented Apr 17, 2018 •
edited
Loading

joyeecheung Apr 19, 2018 •
edited
Loading

BridgeAR commented Apr 25, 2018 •
edited
Loading