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

FR: Use Eslint in CI to directly fix and push style issues into repo #782

Open
Raboro opened this issue Jul 4, 2023 · 8 comments · Fixed by #791
Open

FR: Use Eslint in CI to directly fix and push style issues into repo #782

Raboro opened this issue Jul 4, 2023 · 8 comments · Fixed by #791
Assignees
Labels
github resolution/update-made A change has been made that should resolve this issue or request

Comments

@Raboro
Copy link

Raboro commented Jul 4, 2023

Is Your Feature Request Related to a Problem? Please Describe.

Right now in the main.yml file there is already a linter check setup in line 33: npx elint . --ext .ts. Thats great, however if the check fails the build also fails and it´s requiered to fix the issues manuelly and then push and check again.

Describe the Solution You'd Like

This could be automated so that Eslint already fixes all the issues it could fix inside the action and then push those changes. After that there could be a validate check, where Eslint checks again if now all issues are fixed.

Please include an example where applicable:

Someone pushes into the repo and gets a failed pipeline, because he got some issues. Now these issues must fixed manuelly. So if there a many e.g. 100 issues, this could be really time conuming.

With the auto-fix-push, if the linter detects some issues, those will be fixed and pushed into the repo automaticlly via GH Action Bot. After that there is a check again to validate that there are no issues anymore.

=> Faster development and more automation.

Describe Alternatives You've Considered

I don´t see any alternative.

Additional Context

If not enabled, it must be enabled that a Github Action Bot can push into the repo. For that under Actions > General > allow write and read permission, so the Bot can push the changes.
Those changes are just round about 10 lines of code.
I already done this in my own projects and in a obsidian plugin, works great.

@Raboro Raboro added the rule suggestion Suggestion to add or edit a rule label Jul 4, 2023
@pjkaufman
Copy link
Collaborator

Hey @Raboro . This is a good suggestion. I am just not sure how this would actually get implemented since not all eslint issues are fixable and I am not sure how to prevent an infinite loop in the CI. I will take a closer look at this and see what I can find though.

@Raboro
Copy link
Author

Raboro commented Jul 5, 2023

Yes not all issues are fixable by Eslint, but then the build fails. You implement first a fix with Eslint and commit and push into the repo the fixes with the action. After that you can check again if all issues are fixed and if not the build fails. You are not running into an infinite loop, because the Github action bot push not trigger the pipeline.

@pjkaufman
Copy link
Collaborator

I note that you are saying that the steps to fix the lint issues are manual. However I believe that npm run lint takes care of the issue in question.

Even so, I am looking at if this linting is possible and what that would look like. I appreciate the link to the action.

@pjkaufman
Copy link
Collaborator

I believe I have a fix for this. I have tested it and see it working in the CI. So it will just be a matter of whether or not it can be improved on. I will go ahead and git it out there.

@pjkaufman pjkaufman added github and removed rule suggestion Suggestion to add or edit a rule labels Jul 13, 2023
@pjkaufman pjkaufman self-assigned this Jul 13, 2023
@github-project-automation github-project-automation bot moved this from Backlog to Releasing in Obsidian Linter Jul 13, 2023
@pjkaufman pjkaufman added the resolution/update-made A change has been made that should resolve this issue or request label Jul 13, 2023
@pjkaufman
Copy link
Collaborator

@Raboro , looks like my fix is not an actual fix. It worked for me, however it does not work when it comes to running against forks (https://github.com/stefanzweifel/git-auto-commit-action#use-in-forks-from-public-repositories). I will need to revert the change for now and maybe address this in the future. For now, I would recommend running npm run lint before trying to create a PR.

@pjkaufman
Copy link
Collaborator

The more I am thinking about this, the more I think this should be a local hook to prevent pushing or committing unless lint is passing since the github actions for this have drawbacks when using forks.

@pjkaufman
Copy link
Collaborator

pjkaufman commented Dec 19, 2023

@Raboro , you mentioned that you have an example of this already working in an obsidian plugin. Could you clarify which plugin this is working for already?

Edit: is this it and has it been tested with PRs from outside of the current owner (i.e. from a fork)?

@Raboro
Copy link
Author

Raboro commented Jan 5, 2024

Yes this is the example i mentioned. Trigger Branch is currently only master. But in another project of mine i had a pipeline with more stuff and it worked for PR´s and also all branches. Only forks are not tested, but i think they only need to edit there settings and allow the github action also to write and it should work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
github resolution/update-made A change has been made that should resolve this issue or request
Projects
Status: Backlog
Development

Successfully merging a pull request may close this issue.

2 participants