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

ESLint and Prettier auto-fixes are ignored by the build-flow. #108

Open
smellyshovel opened this issue Jun 20, 2022 · 5 comments
Open

ESLint and Prettier auto-fixes are ignored by the build-flow. #108

smellyshovel opened this issue Jun 20, 2022 · 5 comments
Labels
devops enhancement New feature or request help wanted Extra attention is needed
Milestone

Comments

@smellyshovel
Copy link
Collaborator

In the build-flow there's currently a step that checks and auto-fixes all the code with eslint and prettier. The issue is that the changes are not commited and pushed by the actions bot and therefore they are ignored. This must be addressed at some point 'cause as it is now, it's a waste of resources.

@smellyshovel smellyshovel added this to the v1.x milestone Jun 20, 2022
@wipfli
Copy link
Contributor

wipfli commented Jul 1, 2022

In GL JS, we use eslint to check if everything is fine and if warnings and errors occur, the workflow fails. Then the user just needs to run something manually like npm run lint -- --fix, solve the problem, and commit the change. I prefer this over automatic commits made by the workflow because it is easier to understand for new people, the commits are controlled by a human, and follow-up workflows will run automatically (GitHub has some rules that workflows should not trigger workflows to avoid loops...).

@smellyshovel
Copy link
Collaborator Author

@wipfli sound reasonable enough. But what about the Prettier? It only affects formatting, and I really prefer all the codebase to be style in the same manner. There are no guarantees that each contributor will manually run the Prettier script locally (I even frequently forget to do so myself), so I think it's a good idea to run it during the automated build. What do you think?

@wipfli
Copy link
Contributor

wipfli commented Jul 1, 2022

Yes completely agree. Let's run Prettier in CI and if something is bad, the workflow should fail. We will only merge things with green CI so like this the codebase stays clean...

@smellyshovel
Copy link
Collaborator Author

@wipfli so you mean without auto-fixes on the Prettier side? Just the failing build if something's out of place?

@wipfli
Copy link
Contributor

wipfli commented Jul 1, 2022

Yes

@smellyshovel smellyshovel added enhancement New feature or request help wanted Extra attention is needed devops labels Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devops enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants