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

Feature Request: Only lint modified files #3

Closed
sergeh opened this issue Aug 10, 2021 · 4 comments · Fixed by #9
Closed

Feature Request: Only lint modified files #3

sergeh opened this issue Aug 10, 2021 · 4 comments · Fixed by #9

Comments

@sergeh
Copy link

sergeh commented Aug 10, 2021

Currently, the action runs on all files, it would make more sense to only lint modified liquid files or at least have an optional argument to enable it, something along the lines of Github's Super-Linter

I'm not sure if that's possible considering there are checks like UnusedSnippet that would need to parse the entire codebase.

@charlespwd
Copy link
Contributor

This has been brought up before. Like you said this is a hard problem because we have a bunch of checks that are project wide (unused translations and such).

In the spirit of CI however, should we really make a theme that doesn't check? An alternative approach would be to downgrade the severity of all checks to suggestion until you can bring them back to error.

Then you can gradually make PRs that fixes all the warnings to finally reset the severity to its default value.

In your .theme-check.yml file, you can do this by setting the severity property. Like this:

SyntaxError:
  enabled: true
  ignore: []
  severity: suggestion

There are more details in this section of the README.

@sergeh
Copy link
Author

sergeh commented Aug 12, 2021

In the spirit of CI however, should we really make a theme that doesn't check?

While I do agree with that statement, a regular PR wouldn't normally break things that have already been linted and not modified. In our case, we only lint modified files for features, enhancements, fixes, etc. but we'll lint the entire codebase when preparing a release a PR. It saves a bit of time in terms of action minutes used.

As I said, I realize this may not even be possible and the time saved would probably be negligible on most themes.

@vfonic
Copy link
Contributor

vfonic commented Feb 26, 2022

I regularly work in legacy themes that have tons of errors that I have no intention of ever fixing. It makes more sense to replace the theme altogether than to try to fix all of the issues in the old theme.

I think it would be valuable for end users (theme developers) to be able to choose, via some setting, whether they want a full theme check or just the check/annotations on the changed lines.

Can the tool still perform the same check like now, but just not report on the unmodified lines/files?

@charlespwd
Copy link
Contributor

Hmmmmm. Maybe I can get a list of all the files that were diff'ed and then only push annotations for the ones that are in that list? I'll still do a full theme check but I just won't push annotations for the ones that weren't touched?

Considering the undocumented 1000 annotation limit, this might be required after all.

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 a pull request may close this issue.

3 participants