Skip to content

Conversation

@mstange
Copy link
Contributor

@mstange mstange commented Sep 4, 2025

WIP

## Why silent true?

Because it was hard to find the fix command suggestion in all the output.

```sh
% yarn lint      
yarn run v1.22.22
$ node bin/suggest-fixing-command.js lint-or-lint-fix-internal 'lint --fix'
$ node bin/lint-or-lint-fix.js
$ run-p lint-internal:**
$ node bin/format.js
$ eslint *.js bin src --report-unused-disable-directives --cache --cache-strategy content
$ stylelint "src/**/*.css" "res/**/*.css"
[warn] bin/format.js
[warn] Code style issues found in the above file. Run Prettier with --write to fix.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
ERROR: "lint-internal:format" exited with 1.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
💡 You might be able to fix the error by running `yarn lint --fix`
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
```

## Why format.js?

Because `prettier --check . --fix` doesn't work, it needs to be `prettier --write .`.

## Why so many commands?

Good question.

## What's the mapping of old commands to new commands?

TODO
@codecov
Copy link

codecov bot commented Sep 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.73%. Comparing base (c405844) to head (8dd8df7).
⚠️ Report is 89 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5595   +/-   ##
=======================================
  Coverage   85.73%   85.73%           
=======================================
  Files         309      309           
  Lines       30390    30390           
  Branches     8365     8365           
=======================================
  Hits        26055    26055           
  Misses       3914     3914           
  Partials      421      421           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@canova
Copy link
Member

canova commented Sep 4, 2025

Hmm it looks like a lot of complexity for this. TBH I'm not so sure if it's worth it.

@mstange
Copy link
Contributor Author

mstange commented Sep 4, 2025

I'm planning to write up a bunch of before/after examples in the summary, then we can discuss if it's worth it :)

It looks complex because:

  • one script gets split into three (whose total line count is a bit bigger than the original script's line count, but not by much)
  • a bunch of build-internal commands are added - I'll investigate this again, I think I was solving an issue that was orthogonal to the original intent

Specifically I was annoyed that you got a suggestion to run yarn test -u even though you're already running yarn test -u, if there's a real test failure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants