chore: migrate linting workflow to use trunk check metalinter #17876

EliSchleifer · 2023-12-19T07:20:32Z

Prerequisites checklist

[x ] I have read the contributing guidelines.

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[x ] Other, please explain: chore to migrate linting solution to trunk check - universal metalinter

What changes did you make? (Give an overview)

Trunk is a universal meta-linter and therefore obfuscates the need for separate calls to markdownlint / eslint / etc...

Removed markdownlintignore (trunk by default ignores all files in .gitignore and added separately ignore of changelog.md to trunk.yaml)
All tools will run in batch mode where possible (eslint, markdownlint, svgo)

I've left in place the current git-hook implementation. I want to make sure the basic ergonomics work before migrating these scripts into trunk actions as well.

Is there anything you'd like reviewers to focus on?

Make sure you like the ergonomics of the tool as currently integrated

eslint-github-bot · 2023-12-19T07:20:37Z

Hi @EliSchleifer!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the pull request title to match this format, as we use it to generate changelogs and automate releases.

The length of the commit message must be less than or equal to 72

To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page.

Read more about contributing to ESLint here

linux-foundation-easycla · 2023-12-19T07:20:37Z

✅ login: EliSchleifer / name: Eli Schleifer (f47ca85)
❌ - login: @det / name: Chris Clearwater . The commit (b3ed409) is not authorized under a signed CLA. Please click here to be authorized. For further assistance with EasyCLA, please submit a support request ticket.

netlify · 2023-12-19T07:20:38Z

✅ Deploy Preview for docs-eslint ready!

Name	Link
🔨 Latest commit	`f47ca85`
🔍 Latest deploy log	https://app.netlify.com/sites/docs-eslint/deploys/65f36c9a6733200008a92b56
😎 Deploy Preview	https://deploy-preview-17876--docs-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

eslint-github-bot · 2023-12-19T07:21:05Z

Hi @EliSchleifer!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the pull request title to match this format, as we use it to generate changelogs and automate releases.

The commit message tag wasn't recognized. Did you mean "docs", "fix", or "feat"?
There should be a space following the initial tag and colon, for example 'feat: Message'.
The first letter of the tag should be in lowercase
The length of the commit message must be less than or equal to 72

To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page.

Read more about contributing to ESLint here

nzakas · 2023-12-27T19:06:11Z

Thanks for putting this together. It looks like the CI is failing with a message "trunk not found". Is there a way to get this working in CI to see what the output looks like?

We're working on getting our v9.0.0 alpha out this week, so will have more time to investigate this next week.

EliSchleifer · 2023-12-29T04:28:40Z

Will get that fixed. I’m on vacation till end of the week

…

________________________________ From: Nicholas C. Zakas ***@***.***> Sent: Wednesday, December 27, 2023 11:06:22 AM To: eslint/eslint ***@***.***> Cc: Eli Schleifer ***@***.***>; Mention ***@***.***> Subject: Re: [eslint/eslint] chore: migrate linting workflow to use trunk check metalinter (PR #17876) Thanks for putting this together. It looks like the CI is failing with a message "trunk not found". Is there a way to get this working in CI to see what the output looks like? We're working on getting our v9.0.0 alpha out this week, so will have more time to investigate this next week. — Reply to this email directly, view it on GitHub<#17876 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAJVCPVJGWDYDW6UV4GCXBLYLRWS5AVCNFSM6AAAAABA2WKCSOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZQGU2TQMZUGU>. You are receiving this because you were mentioned.Message ID: ***@***.***>

nzakas · 2024-01-04T20:11:05Z

.trunk/trunk.yaml

+ - name: lint
+ run: ${workspace}/bin/eslint.js --output-file ${tmpfile} --format json ${target}
+ enabled:
+ - [email protected]


I'm curious if this is needed given the previous definition?

Yeah - the definition teaches trunk how to run a tool - and the enabled list turns on the tools.

nzakas · 2024-01-04T20:11:45Z

.trunk/trunk.yaml

+ - trunk-check-pre-push
+ - trunk-fmt-pre-commit
+ enabled:
+ - trunk-upgrade-available


Question: Will something like Renovate know to upgrade trunk? Or is this something that needs to happen manually?

Currently upgrading is manual - we are working on a PR to make renovate aware of our schema.

nzakas · 2024-01-04T20:12:55Z

package.json

@@ -30,7 +30,8 @@
 "test": "node Makefile.js test",
 "test:cli": "mocha",
 "test:fuzz": "node Makefile.js fuzz",
- "test:performance": "node Makefile.js perf"
+ "test:performance": "node Makefile.js perf",
+ "trunk": "trunk"


I'm thinking we'd probably want "lint": "trunk" if the intent is to replace the current linting task?

yeah - I just need to extract the json file check and then we're in business

EliSchleifer · 2024-01-09T17:57:49Z

@nzakas - I think this cleans up all the issues and at least let's you kick tires completely. I didn't see an active GitHub action that ran ESLint on itself - is the lint step right now fully optional?

I can add a trunk check step that would automatically run check on each PR and make sure all rules are being followed.

nzakas · 2024-01-09T18:13:20Z

Because you're a first-time contributor, the lint check doesn't run automatically -- I just manually approved it so it will run now. It's titled "Verify Files" in the actions list.

nzakas · 2024-01-09T18:16:08Z

d

This looks like it was included accidentally?

nzakas · 2024-01-09T18:17:37Z

.trunk/trunk.yaml

+ enabled:
+ - eslint
+ - [email protected]
+ - [email protected]
+ - git-diff-check
+ - [email protected]
+ - [email protected]
+ - [email protected]
+ - [email protected]
+ - [email protected]
+ - [email protected]
+ - [email protected]
+ - [email protected]
+ - [email protected]
+ - [email protected]


Do these versions correspond to the versions in package.json? I'm just concerned about needing to update two places when these change.

...or do these have no relation to package.json at all?

These have no relation to package.json - I don't think you had/have versions of these tools running there. Some of these tools are nice to have - svgo, oxipng, shfmt, yamllint - nice tools to keep things clean and tidy.

Did you see any tool overlap - agreed - we don't want to have any collision with package.json

nzakas · 2024-01-10T18:17:02Z

d

Can we remove this file?

Geeze - that's a ridiculous file.

nzakas · 2024-01-11T18:10:42Z

I've tried checking this out locally and on my Windows machine I'm getting the following:

$ npm run trunk

> [email protected] trunk
> trunk

The system cannot find the path specified.

I tried running npm install, but it seems that doesn't help. Does trunk work on Windows? Is there something else I need to do get it to work locally?

EliSchleifer · 2024-01-11T19:32:12Z

I've tried checking this out locally and on my Windows machine I'm getting the following:
$ npm run trunk

> [email protected] trunk
> trunk

The system cannot find the path specified.
I tried running npm install, but it seems that doesn't help. Does trunk work on Windows? Is there something else I need to do get it to work locally?

The product does work on windows - but not sure we've tested the npm flow - I am not a node/windows expert - but we have those on staff. They're taking a look.

EliSchleifer · 2024-01-11T22:55:34Z

two issues here:

I mistakenly changed call to trunk check from CI even though trunk was not on the path - so moved to use npm run
You uncovered a bug in our launcher for windows - having someone from the team fix that up. @nzakas - do you use powershell on your windows box or bash emulation?

nzakas · 2024-01-16T20:56:32Z

I'm using Git Bash.

github-actions · 2024-01-26T22:31:14Z

Hi everyone, it looks like we lost track of this pull request. Please review and see what the next steps are. This pull request will auto-close in 7 days without an update.

JoshuaKGoldberg · 2024-01-31T18:58:33Z

.trunk/configs/.yamllint.yaml

There also exists an eslint-plugin-yml. I suspect it would be good community-dogfooding for ESLint to use that rather than a separate tool. Is that something that can be reasonably done in metalinter without sacrificing features?

Let me take this as an action item for a follow up to use the eslint yaml plugin.

nzakas · 2024-02-06T17:39:52Z

@EliSchleifer looks like we have the CI failing again on this.

EliSchleifer · 2024-02-06T23:56:44Z

on it - also looking into how we can better support the mode of running on docs only in CI and not locally. small product change to make that a better experience and map to what we're aiming for here

@EliSchleifer looks like we have the CI failing again on this.

github-actions · 2024-02-17T22:31:16Z

Hi everyone, it looks like we lost track of this pull request. Please review and see what the next steps are. This pull request will auto-close in 7 days without an update.

Tanujkanti4441 · 2024-02-18T13:34:28Z

hi @EliSchleifer, you can share the updates regarding this PR or if it is ready to review.

EliSchleifer · 2024-02-20T04:38:37Z

I need to update the config - so the verification step is properly modeling the desire to handle CI flow differently from PR local flow. I should be able to update it this week

nzakas · 2024-02-27T17:36:18Z

.github/workflows/ci.yml

@@ -33,9 +33,6 @@ jobs:
 - name: Check Licenses
 run: node Makefile checkLicenses

- - name: Lint Docs JS Files


We wanted this a separate step so we could more easily tell where a failure was on a PR.

The GitHub action from trunk will annotate the files inline with the issue. Is that not sufficient to get an understanding of the source of the problem.

I can create a sub branch to demonstrate

Will those show up on the Discussion tab of the PR? What I'm looking for is the "glance-ability" of a PR so I don't have to switch to the code tab to get a general understanding which CI checks failed. This is key when we are barraged with PRs and trying to triage.

ping @EliSchleifer

github-actions · 2024-03-08T22:31:18Z

Hi everyone, it looks like we lost track of this pull request. Please review and see what the next steps are. This pull request will auto-close in 7 days without an update.

EliSchleifer · 2024-03-14T03:34:36Z

Got a plan to make this work beautifully. Will post update tomorrow morning

________________________________ From: Joel Mathew Koshy ***@***.***> Sent: Wednesday, March 13, 2024 7:19:07 PM To: eslint/eslint ***@***.***> Cc: Eli Schleifer ***@***.***>; Mention ***@***.***> Subject: Re: [eslint/eslint] chore: migrate linting workflow to use trunk check metalinter (PR #17876) @Rec0iL99 commented on this pull request.

________________________________ In .github/workflows/ci.yml<#17876 (comment)>:

@@ -33,9 +33,6 @@ jobs:

- name: Check Licenses run: node Makefile checkLicenses - - name: Lint Docs JS Files ping @EliSchleifer<https://github.com/EliSchleifer> — Reply to this email directly, view it on GitHub<#17876 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAJVCPQ3LQQPFRPJPRPQOZ3YYECJXAVCNFSM6AAAAABA2WKCSOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTSMZVGUZDSMJVGQ>. You are receiving this because you were mentioned.Message ID: ***@***.***>

fasttime · 2024-05-01T18:10:51Z

@EliSchleifer there are merge conflicts. Can you take a look?

EliSchleifer changed the title ~~chore: migrate linting / static analysis workflow to unified trunk metalinter~~ Chore: migrate linting / static analysis workflow to unified trunk metalinter Dec 19, 2023

EliSchleifer changed the title ~~Chore: migrate linting / static analysis workflow to unified trunk metalinter~~ chore: migrate linting workflow to use trunk check metalinter Dec 19, 2023

eslint-github-bot bot added the chore This change is not user-facing label Dec 19, 2023

nzakas reviewed Jan 4, 2024

View reviewed changes

EliSchleifer marked this pull request as ready for review January 9, 2024 17:56

EliSchleifer requested a review from a team as a code owner January 9, 2024 17:56

nzakas reviewed Jan 9, 2024

View reviewed changes

nzakas reviewed Jan 10, 2024

View reviewed changes

github-actions bot added the Stale label Jan 26, 2024

Tanujkanti4441 removed the Stale label Jan 27, 2024

nzakas mentioned this pull request Jan 31, 2024

Change Request: Consider adopting prettier internally #17814

Open

1 task

JoshuaKGoldberg reviewed Jan 31, 2024

View reviewed changes

github-actions bot added the Stale label Feb 17, 2024

Tanujkanti4441 removed the Stale label Feb 18, 2024

github-actions bot added the github actions label Feb 27, 2024

nzakas reviewed Feb 27, 2024

View reviewed changes

github-actions bot added the Stale label Mar 8, 2024

Tanujkanti4441 removed the Stale label Mar 9, 2024

Tanujkanti4441 assigned EliSchleifer Mar 9, 2024

Tanujkanti4441 mentioned this pull request Mar 14, 2024

chore: Testing PR flows #18203

Closed

1 task

b3ed409

EliSchleifer force-pushed the main branch from 6969bf9 to b3ed409 Compare March 14, 2024 21:24

remove test file

f47ca85

chore: migrate linting workflow to use trunk check metalinter #17876

Are you sure you want to change the base?

chore: migrate linting workflow to use trunk check metalinter #17876

Conversation

EliSchleifer commented Dec 19, 2023

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

What changes did you make? (Give an overview)

Is there anything you'd like reviewers to focus on?

eslint-github-bot bot commented Dec 19, 2023

linux-foundation-easycla bot commented Dec 19, 2023 • edited

netlify bot commented Dec 19, 2023 • edited

✅ Deploy Preview for docs-eslint ready!

eslint-github-bot bot commented Dec 19, 2023

nzakas commented Dec 27, 2023

EliSchleifer commented Dec 29, 2023 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EliSchleifer commented Jan 9, 2024

nzakas commented Jan 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nzakas commented Jan 11, 2024

EliSchleifer commented Jan 11, 2024

EliSchleifer commented Jan 11, 2024

nzakas commented Jan 16, 2024

github-actions bot commented Jan 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nzakas commented Feb 6, 2024

EliSchleifer commented Feb 6, 2024

github-actions bot commented Feb 17, 2024

Tanujkanti4441 commented Feb 18, 2024

EliSchleifer commented Feb 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 8, 2024

EliSchleifer commented Mar 14, 2024 via email

fasttime commented May 1, 2024

linux-foundation-easycla bot commented Dec 19, 2023 •

edited

netlify bot commented Dec 19, 2023 •

edited