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 a "fix" option #7

Open
aparlato opened this issue Jan 9, 2023 · 3 comments
Open

Add a "fix" option #7

aparlato opened this issue Jan 9, 2023 · 3 comments

Comments

@aparlato
Copy link
Contributor

aparlato commented Jan 9, 2023

Right now this repo is focused specifically on linting the style input.

Should we add a --fix flag option to correct instances the linter warns about?

In client specific implementations of clean scripts, we have custom written scripts to correct for single-match expressions which are linted by this repo.

If we add a flag to fix those instances we would no longer need bespoke implementations of that cleanup script.

That said, I wonder how we might let users pick and choose which things they want to fix. For example, this repo will warn when you have a single match expression and when you have a duplicate output from an expression. I can think of instances where an author wants to correct the single match expression but not the duplicate output as its a stylistic choice put there for legibility reasons.

Is this exception worth considering? If so, how might we proceed?

cc @ebrelsford

@ebrelsford
Copy link
Contributor

I'm open to it. I agree with the exception you listed.

We're also generally using this linter on styles built with the build system and it seems like ideally we'd be fixing these issues at the source (in the layer templates) rather than on the built files, right?

@kelsey-taylor
Copy link
Collaborator

I'm open to it. I agree with the exception you listed. We're also generally using this linter on styles built with the build system and it seems like ideally we'd be fixing these issues at the source (in the layer templates) rather than on the built files, right?

i'd love to find a way to address the latter, although that feels like a slightly separate topic. not sure how easy/hard this would be with variables but at least some of it (removing defaults) feels doable

@aparlato
Copy link
Contributor Author

aparlato commented Jan 9, 2023

We're also generally using this linter on styles built with the build system and it seems like ideally we'd be fixing these issues at the source (in the layer templates) rather than on the built files, right?

Yea that's a good point, we do generally want to fix these on the source so this should be a lower priority. That said, we've written scripts to clean styles like this even as a first round before turning them into build system templates, so I think this would still be helpful to avoid custom scripts.

i'd love to find a way to address the latter, although that feels like a slightly separate topic. not sure how easy/hard this would be with variables but at least some of it (removing defaults) feels doable

Yea, if I'm reading this right, I don't think it'll be worth the lift to try and rewrite template files automatically. This would only really benefit us:

  • before template-ifying styles
  • as an interim solution before we make source file changes to run after build every time

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

No branches or pull requests

3 participants