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.

Sign up for GitHub

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

Jump to bottom

doc: document make lint-md-clean #47926

Merged
merged 1 commit into from
May 10, 2023
Merged

doc: document make lint-md-clean #47926

merged 1 commit into from
May 10, 2023

Conversation

mcollina
Copy link
Member

@mcollina mcollina commented May 8, 2023

I've spent quite a bit of time debugging why the lining was failing locally.
I thought it would be useful to document the make lint-md-clean job.

@mcollina
doc: document make lint-md-clean
030e662 
Signed-off-by: Matteo Collina <hello@matteocollina.com>
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/tsc

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label May 8, 2023
@Trott
Copy link
Member

Trott commented May 8, 2023

I've spent quite a bit of time debugging why the lining was failing locally. I thought it would be useful to document the make lint-md-clean job.

That's odd. The only way this could happen is if the timestamp for errors.md is older than the timestamp for the linting job. Maybe something weird happened when changing branches or something like that?

FWIW, the line in the Makefile for lint-md-clean where it removes the node_modules directory is obsolete. The node_modules directory it removes doesn't ever get created anymore because we use rollup. The part where it removes the timestamp file, though, is still valid.

I wonder if we should also document make lint-clean which removes caching for both the md-linting and the js-linting.

None of these comments are blocking. Change LGTM.

@jasnell jasnell added the fast-track PRs that do not need to wait for 48 hours to land. label May 10, 2023
@github-actions
Copy link
Contributor

Fast-track has been requested by @jasnell. Please 👍 to approve.

@MoLow MoLow added the commit-queue Add this label to land a pull request using GitHub Actions. label May 10, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 10, 2023
@nodejs-github-bot nodejs-github-bot merged commit ded6b61 into nodejs:main May 10, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in ded6b61

targos pushed a commit that referenced this pull request May 12, 2023
@mcollina @targos
doc: document make lint-md-clean
c9ffc55 
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: #47926
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@targos targos mentioned this pull request May 15, 2023
Merged
danielleadams pushed a commit that referenced this pull request Jul 6, 2023
@mcollina @danielleadams
doc: document make lint-md-clean
eecb889 
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: #47926
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MoLow pushed a commit to MoLow/node that referenced this pull request Jul 6, 2023
@mcollina @MoLow
doc: document make lint-md-clean
2cc8715 
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: nodejs#47926
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@danielleadams danielleadams mentioned this pull request Jul 10, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RafaelGSS RafaelGSS RafaelGSS approved these changes

@tniessen tniessen tniessen approved these changes

@anonrig anonrig anonrig approved these changes

@Trott Trott Trott approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
doc Issues and PRs related to the documentations. fast-track PRs that do not need to wait for 48 hours to land.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@mcollina @nodejs-github-bot @Trott @jasnell @anonrig @tniessen @RafaelGSS @MoLow