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

ci: update to not auto fix lint errors #256

Merged
merged 3 commits into from
Apr 27, 2022
Merged

ci: update to not auto fix lint errors #256

merged 3 commits into from
Apr 27, 2022

Conversation

kgajera
Copy link
Contributor

@kgajera kgajera commented Apr 8, 2022

Changes

  • Updates yarn lint to not auto fix lint errors. This was being run in the CI, so the lint step would pass even when lint errors exist.
  • Adds new yarn lint:fix script that can auto fix lint errors
  • Updates the generated GraphQL typings file to not be linted anymore because this was added to the gitignore a while ago so there's no point in linting it.
  • Upgrades graphql codegen dependencies

Checklist

  • Requires dependency update?
  • Generating a new app works

Copy link
Contributor

@code-jenn-or code-jenn-or left a comment

Choose a reason for hiding this comment

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

Thanks for continuing to suggest improvements.

@@ -32,7 +32,8 @@
"g:test:factory": "hygen test factory --name",
"g:test:request": "hygen test request --name",
"g:test:util": "hygen test util --name",
"lint": "yarn eslint . --ext .ts,.tsx --fix --ignore-pattern tmp",
"lint": "yarn eslint . --ext .ts,.tsx --ignore-pattern tmp",
"lint:fix": "yarn eslint . --ext .ts,.tsx --fix --ignore-pattern tmp",
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer this separation! Great add to not just pull out the ability to fix, but to create another distinct script.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you want to make sure both scripts do the same thing, except for fixing lint errors, you could do:

lint:fix": "yarn lint --fix"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks Cully. Made this chnage.

Copy link
Contributor

@cullylarson cullylarson left a comment

Choose a reason for hiding this comment

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

Great idea, Kishan! I like it.

@@ -32,7 +32,8 @@
"g:test:factory": "hygen test factory --name",
"g:test:request": "hygen test request --name",
"g:test:util": "hygen test util --name",
"lint": "yarn eslint . --ext .ts,.tsx --fix --ignore-pattern tmp",
"lint": "yarn eslint . --ext .ts,.tsx --ignore-pattern tmp",
"lint:fix": "yarn eslint . --ext .ts,.tsx --fix --ignore-pattern tmp",
Copy link
Contributor

Choose a reason for hiding this comment

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

If you want to make sure both scripts do the same thing, except for fixing lint errors, you could do:

lint:fix": "yarn lint --fix"

@kgajera kgajera merged commit 3c4c5c8 into canary Apr 27, 2022
@kgajera kgajera deleted the refactor/lint branch April 27, 2022 15:56
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.

3 participants