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

fix: remark-lint, caches and preset-lint-node #6006

Merged
merged 2 commits into from
Oct 16, 2023

Conversation

ovflowd
Copy link
Member

@ovflowd ovflowd commented Oct 15, 2023

Description

This PR is pretty much (is a second attempt) at working on some of our remark-lint rules. (Based on the prior merged PR (#5999), I've realized that not adopting remark-preset-lint-node as a "compatibility issue" is an excuse.

I've read a few docs and tried to play around with a few things.

  • First, we shouldn't cache lint rules with Turborepo. Even more, as this repository is ever-changing, it only feels that having a separate file, turbo.json, to keep track of every single file does feel like something at best flaky and waiting to break apart.
  • Going through Node.js's Website repository history, I've found a set of Remark settings on .remarkc that actually explain the misformatting of Frontmatter when eslint-plugin-mdx was attempting to run Remark Lint on files. Apparently, just by removing remark-lint-preset-node, the issue itself wouldn't have been fixed.
  • I've found that we can actually adopt the remark-preset-lint-node with a few tweaks, such as manually disabling remark-gfm (as we don't need it) (and apparently on the repository history many eons ago it was at some point disabled). (History often taught us lessons!)
  • I've found that even tho locally eslint and prettier caching with metadata approach is way faster, apparently on CI it isn't (and actually completely ineffective) due to Git of course not storing timestamp metadata 🤦 (my bad!)
    • Switching to content checks should be a little bit slower, but still way faster than current times on CI!

This PR in the end ships some CI speed improvements for caching, Better Remark Linting configuration, and enabled Stylelint to also use its cache feature.

Validation

Linting should pass as usual (Note that some files got updated).

@ovflowd ovflowd requested review from a team as code owners October 15, 2023 14:57
@vercel
Copy link

vercel bot commented Oct 15, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nodejs-org ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 15, 2023 11:16pm

@ovflowd ovflowd added the infrastructure Issues/PRs related to the Repository Infra label Oct 15, 2023
@github-actions
Copy link

Note

Your Pull Request seems to be updating Translations of the Node.js Website.

Whilst we appreciate your intent; Any Translation update should be done through our Crowdin Project.
We recommend giving a read on our Translation Guidelines.

Thank you!

@github-actions
Copy link

github-actions bot commented Oct 15, 2023

Unit Test Coverage Report

Lines Statements Branches Functions
Coverage: 96%
95.19% (297/312) 77.77% (56/72) 93.75% (60/64)

Unit Test Report

Tests Skipped Failures Errors Time
31 0 💤 0 ❌ 0 🔥 6.251s ⏱️

@bmuenzenmeyer
Copy link
Collaborator

I want to dig a bit further into https://github.com/nodejs/remark-preset-lint-node before weighing in

Copy link
Collaborator

@bmuenzenmeyer bmuenzenmeyer left a comment

Choose a reason for hiding this comment

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

small question about the persist keys the in turbo.json

.gitignore Show resolved Hide resolved
.remarkrc.json Show resolved Hide resolved
turbo.json Outdated Show resolved Hide resolved
@ovflowd
Copy link
Member Author

ovflowd commented Oct 15, 2023

I want to dig a bit further into nodejs/remark-preset-lint-node before weighing in

Right, take your time :)

Copy link
Collaborator

@bmuenzenmeyer bmuenzenmeyer left a comment

Choose a reason for hiding this comment

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

I appreciate your commitment and drive to get this right, even if we don't always know what that is in the moment. Progress over perfection, and humility to go back and fix things 😄

sheplu

This comment was marked as duplicate.

Copy link
Member

@sheplu sheplu left a comment

Choose a reason for hiding this comment

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

LGTM

@bmuenzenmeyer bmuenzenmeyer added this pull request to the merge queue Oct 16, 2023
Merged via the queue into main with commit a196a48 Oct 16, 2023
16 checks passed
@bmuenzenmeyer bmuenzenmeyer deleted the meta/fix-cache-linting-remark-lint branch October 16, 2023 03:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fast-track Fast Tracking PRs infrastructure Issues/PRs related to the Repository Infra
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants