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

Add fix/lint helper dev targets #23561

Merged
merged 3 commits into from
Jun 1, 2023
Merged

Conversation

nschonni
Copy link
Contributor

@nschonni nschonni commented Feb 13, 2023

ESLint and Stylelint both allow a --fix parameter to autofix rules, but things like Prettier use another --write command.
This adds consistent targets so developers can use the tool's autofixing easily, along with a rollup yarn fix to run all autofixes.

@github-actions
Copy link
Contributor

This pull request has merge conflicts that must be resolved before it can be merged.

@github-actions
Copy link
Contributor

This pull request has merge conflicts that must be resolved before it can be merged.

package.json Outdated Show resolved Hide resolved
@nschonni
Copy link
Contributor Author

I updated this to include Markdown targets after #21972 landed, but realized I missed a trigger there, so added in a slightly unrelated commit to fix that

@nschonni
Copy link
Contributor Author

nschonni commented Mar 7, 2023

If there is a preference to go with the shorter names like in #23920 I can flatter them down to lint:* / fix:* instead.

package.json Outdated
"start": "node ./streaming/index.js",
"test": "${npm_execpath} run test:lint:js && ${npm_execpath} run test:jest",
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Is there a reason you are replacing ${npm_execpath} with yarn?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It wasn't a pattern I had come across before, and didn't seem to be cross-platform compatible as it relies on bash shell expansion

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

I don't think && is cross platform in the first place.

Copy link
Contributor Author

@nschonni nschonni Mar 9, 2023

Choose a reason for hiding this comment

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

&& is cross platform. Here is a litte example, first run I cut short, but run on this branch, then switched back to main show the error

PS Z:\mastodon> yarn --version
1.22.19
PS Z:\mastodon> yarn lint
yarn run v1.22.19
$ yarn lint:js && yarn lint:json && yarn lint:sass && yarn lint:yml
$ eslint . --ext .js,.jsx --cache --report-unused-disable-directives

Z:\mastodon\app\javascript\mastodon\components\status.jsx
  517:13  warning  Avoid non-native interactive elements. If using native HTML is not possible, add an appropriate role and support for tabbing, mouse, keyboard, and touch inputs to an interactive content element  jsx-a11y/no-static-element-interactions

✖ 1 problem (0 errors, 1 warning)

$ prettier --check "**/*.json"
Checking formatting...
All matched files use Prettier code style!
$ stylelint "**/*.{css,scss}" && prettier --check "**/*.{css,scss}"
PS Z:\mastodon> yarn test:lint
yarn run v1.22.19
$ ${npm_execpath} run test:lint:js && ${npm_execpath} run test:lint:sass
'${npm_execpath}' is not recognized as an internal or external command,
operable program or batch file.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
PS Z:\mastodon>

Same with gitbash on windows

$ yarn test:lint
yarn run v1.22.19
$ ${npm_execpath} run test:lint:js && ${npm_execpath} run test:lint:sass
'${npm_execpath}' is not recognized as an internal or external command,
operable program or batch file.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

package.json Outdated Show resolved Hide resolved
@nschonni nschonni changed the title Add test:lint:*:fix helper dev targets Add fix/lint helper dev targets Mar 9, 2023
@nschonni nschonni force-pushed the test-lint-fix branch 2 times, most recently from 152356c to 7ca8ba6 Compare March 9, 2023 00:42
@nschonni
Copy link
Contributor Author

nschonni commented Mar 9, 2023

I flattened out the target names to just lint:* and fix:*

@nschonni
Copy link
Contributor Author

@ykzts any more concerns with this one?

@github-actions
Copy link
Contributor

github-actions bot commented Apr 3, 2023

This pull request has merge conflicts that must be resolved before it can be merged.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 3, 2023

This pull request has resolved merge conflicts and is ready for review.

package.json Outdated Show resolved Hide resolved
@nschonni
Copy link
Contributor Author

nschonni commented Apr 3, 2023

I updated the js-linting CI to call the test:typecheck and the expanded file globs, but I could split that out to a separate PR if it's easier to land that way

@github-actions
Copy link
Contributor

github-actions bot commented Apr 4, 2023

This pull request has merge conflicts that must be resolved before it can be merged.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 4, 2023

This pull request has resolved merge conflicts and is ready for review.

@renchap renchap added javascript Pull requests that update Javascript code lint fix 💅 labels May 3, 2023
@github-actions
Copy link
Contributor

github-actions bot commented May 4, 2023

This pull request has merge conflicts that must be resolved before it can be merged.

@github-actions
Copy link
Contributor

github-actions bot commented May 4, 2023

This pull request has resolved merge conflicts and is ready for review.

@github-actions
Copy link
Contributor

This pull request has merge conflicts that must be resolved before it can be merged.

@github-actions
Copy link
Contributor

This pull request has resolved merge conflicts and is ready for review.

@Gargron Gargron merged commit d39bce9 into mastodon:main Jun 1, 2023
29 checks passed
@nschonni nschonni deleted the test-lint-fix branch June 1, 2023 04:21
supernovae pushed a commit to Universeodon-com/mastodon that referenced this pull request Jun 7, 2023
skerit pushed a commit to 11ways/mastodon that referenced this pull request Jul 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
javascript Pull requests that update Javascript code lint fix 💅
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants