GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — tools,doc: bump remark to 13.0.0 for markdown linter rollup by Trott · Pull Request #35905 · nodejs/node

Conversation

@Trott

@Trott Trott commented Nov 1, 2020

Copy link
Copy Markdown
Member
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bricss reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@nodejs-github-bot

nodejs-github-bot commented Nov 1, 2020

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/crypto
  • @nodejs/modules
  • @nodejs/net
  • @nodejs/quic
  • @nodejs/tsc
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. labels Nov 1, 2020
@targos

targos commented Nov 1, 2020

Copy link
Copy Markdown
Member

All those disable comments suggest that remark should maybe ignore tables or have an option for that
tniessen and DerekNonGeneric reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
fhinkel
fhinkel approved these changes Nov 2, 2020
View reviewed changes
@targos

targos commented Nov 2, 2020

Copy link
Copy Markdown
Member

https://github.com/remarkjs/remark/releases/tag/13.0.0

The changelog says that for tables we need remark-gfm. Do we have it?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
codebytere
codebytere approved these changes Nov 2, 2020
View reviewed changes
targos
targos suggested changes Nov 2, 2020
View reviewed changes

@targos targos left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Requesting changes so my question is not missed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@Trott

This comment has been minimized.

Sign in to view
@Trott

This comment has been minimized.

Sign in to view
@Trott

Trott commented Nov 2, 2020

Copy link
Copy Markdown
Member Author

https://github.com/remarkjs/remark/releases/tag/13.0.0
The changelog says that for tables we need remark-gfm. Do we have it?

That's if you're using remark to render markdown to HTML. Here, we're using it to lint only. As far as I can tell, remark-gfm is unused in remark-lint.

Oops nope, I'm wrong.

All those disable comments suggest that remark should maybe ignore tables or have an option for that

This would seem to be either a bug or a new feature in remark-lint-maximum-line-length as the README currently says it is supposed to exclude tables. I'm guessing that the code to detect/exclude lines in tables is somehow broken when using remark@13 to parse.

And oops nope, I'm wrong here too.

Will get GFM enabled. Might involve updating remark-preset-lint-node though.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Trott

Trott commented Nov 3, 2020

Copy link
Copy Markdown
Member Author

This is blocked on #35934.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Trott Trott added the blocked PRs that are blocked by other issues or PRs. label Nov 3, 2020
@Trott
Trott force-pushed the remark-13 branch from 8e1ceaf to 15ffe05 Compare November 3, 2020 00:38
@Trott

Trott commented Nov 3, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

@targos Your comments have been addressed. While the comments were spot-on, unfortunately they also mean this will now fail until:

  • doc: adjust tables to prepare for stricter linting #35934 lands
  • https://github.com/nodejs/remark-preset-lint-node/pull/161/files lands
  • a new version of remark-preset-lint-node is released
  • that new version is incorporated into this PR

Those four things must happen in that order.
targos reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Trott
Trott force-pushed the remark-13 branch from 15ffe05 to 02c94c2 Compare November 3, 2020 13:03
@Trott

Trott commented Nov 3, 2020

Copy link
Copy Markdown
Member Author

OK, all done. PTAL, especially @targos!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Trott Trott mentioned this pull request Nov 3, 2020
chore: update CI linting nodejs/remark-preset-lint-node#162
Merged
targos
targos approved these changes Nov 3, 2020
View reviewed changes
Trott added a commit that referenced this pull request Nov 3, 2020
@Trott
doc: update tables in README files for linting changes
adfcade 
PR-URL: #35905
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@Trott

Trott commented Nov 3, 2020

Copy link
Copy Markdown
Member Author

Landed in 1a29c09...1bc1f84
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Trott Trott closed this Nov 3, 2020
@Trott
Trott force-pushed the remark-13 branch from 02c94c2 to 1bc1f84 Compare November 3, 2020 13:19
@Trott
Trott deleted the remark-13 branch November 3, 2020 13:19
@Trott
Trott merged commit 1bc1f84 into nodejs:master Nov 3, 2020
targos pushed a commit that referenced this pull request Nov 3, 2020
@Trott @targos
doc: update tables in README files for linting changes
7937fbe 
PR-URL: #35905
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
targos pushed a commit that referenced this pull request Nov 3, 2020
@Trott @targos
tools: bump remark-lint-preset-node to 2.0.0
fd027cd 
Update [email protected], [email protected] and other
dependencies in the lint-md rollup.

PR-URL: #35905
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@targos targos mentioned this pull request Nov 3, 2020
v15.1.0 release proposal #35948
Merged
BethGriggs pushed a commit that referenced this pull request Dec 8, 2020
@Trott @BethGriggs
doc: update tables in README files for linting changes
4907af1 
PR-URL: #35905
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
BethGriggs pushed a commit that referenced this pull request Dec 10, 2020
@Trott @BethGriggs
doc: update tables in README files for linting changes
5c3e56b 
PR-URL: #35905
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@BethGriggs BethGriggs mentioned this pull request Dec 10, 2020
v14.15.2 proposal #36476
Merged
BethGriggs pushed a commit that referenced this pull request Dec 15, 2020
@Trott @BethGriggs
doc: update tables in README files for linting changes
51ce1a2 
PR-URL: #35905
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
targos pushed a commit that referenced this pull request May 1, 2021
@Trott @targos
tools: bump remark-lint-preset-node to 2.0.0
ac70a59 
Update [email protected], [email protected] and other
dependencies in the lint-md rollup.

PR-URL: #35905
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@danielleadams danielleadams mentioned this pull request May 3, 2021
v14.17.0 proposal #38507
Merged
danielleadams pushed a commit that referenced this pull request May 8, 2021
@Trott @danielleadams
tools: bump remark-lint-preset-node to 2.0.0
7715fb4 
Update [email protected], [email protected] and other
dependencies in the lint-md rollup.

PR-URL: #35905
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@codebytere codebytere codebytere approved these changes
+2 more reviewers
@fhinkel fhinkel fhinkel approved these changes
@targos targos targos approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

blocked PRs that are blocked by other issues or PRs. c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@Trott @nodejs-github-bot @targos @fhinkel @codebytere
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.