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: host remark-preset-lint-node in-tree #17441

Closed
wants to merge 1 commit into from

Conversation

maclover7
Copy link
Contributor

Moved from https://github.com/watilde/remark-preset-lint-node. The source for this preset will now live in the nodejs/node repository.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

tools

@nodejs-github-bot nodejs-github-bot added the tools Issues and PRs related to the tools directory. label Dec 3, 2017
@Trott
Copy link
Member

Trott commented Dec 4, 2017

Now that I'm seeing the change set, it makes me want to ask: What's the advantage of doing it this way?

@maclover7
Copy link
Contributor Author

@Trott main advantages are that there's no need to publish versions on npm or worry about staying up to date with a separate repo -- basically the same deal we have with the docs generator tool.

@BridgeAR
Copy link
Member

@nodejs/collaborators thoughts?

@watilde
Copy link
Member

watilde commented Dec 12, 2017

Initially, it was an independent private package when I added so we didn't need to worry about the version of the remark-preset-lint-node on npm registry. It might be worth to use the idea again.

Also, it's totally fine to transfer the remark-preset-lint-node repository under @nodejs org if we want :)

// Sorry for my late response!

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

LGTM

@maclover7
Copy link
Contributor Author

@nodejs/collaborators any thoughts? With this the project markdown lint rules will live inside nodejs/node and not in a separate repo. They are already checked into the repo (see the tools/remark-preset-lint-node directory), this just makes it the official source.

@BridgeAR
Copy link
Member

BridgeAR commented Jan 5, 2018

Landed in a9e422e

@BridgeAR BridgeAR closed this Jan 5, 2018
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Jan 5, 2018
Moved from https://github.com/watilde/remark-preset-lint-node.

PR-URL: nodejs#17441
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@maclover7 maclover7 deleted the jm-mv-preset branch January 5, 2018 02:42
MylesBorins pushed a commit that referenced this pull request Jan 8, 2018
Moved from https://github.com/watilde/remark-preset-lint-node.

PR-URL: #17441
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this pull request Jan 9, 2018
Moved from https://github.com/watilde/remark-preset-lint-node.

PR-URL: #17441
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this pull request Jan 9, 2018
Moved from https://github.com/watilde/remark-preset-lint-node.

PR-URL: #17441
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@MylesBorins MylesBorins mentioned this pull request Jan 10, 2018
@MylesBorins
Copy link
Contributor

This doesn't land cleanly on LTS. please feel free to backport. Marked as don't land for now

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.

8 participants