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 generic lint GitHub actions #938

Closed
wants to merge 1 commit into from
Closed

Add generic lint GitHub actions #938

wants to merge 1 commit into from

Conversation

dnkrj
Copy link

@dnkrj dnkrj commented Aug 31, 2023

What

  • Add actions for yarn run lint:js, yarn run lint:scss and yarn run lint:prettier

Why

  • We're introducing new linting to Whitehall, Use standard for linting JavaScript whitehall#8178, based on govuk-frontend
  • The existing lint actions specify the exact command to run
  • These new actions allow applications to configure their own linting whilst still running in parallel

The existing lint actions specify the exact command to run, these new actions allow applications to configure their own linting whilst still running in parallel
@theseanything
Copy link
Contributor

I'm wondering what the value is of having these as reusable workflows vs just writing the individual steps in calling workflow (e.g. CI for Whitehall)? They are essential alias commands to running other tools, so limited in the configuration we can have here.

@dnkrj
Copy link
Author

dnkrj commented Aug 31, 2023

I'm wondering what the value is of having these as reusable workflows vs just writing the individual steps in calling workflow (e.g. CI for Whitehall)? They are essential alias commands to running other tools, so limited in the configuration we can have here.

I'd be open to doing that as well.

My concern would be that it makes it difficult to add or remove steps in future:
Ideally a new step is defined here first and then referenced in the application repo when it is ready to use it (lint:prettier in this PR for example).
In the case where we have a single action for Whitehall, the application repo would need to be updated first and then the action here - leading to a situation where CI in Whitehall could be more easily broken by a PR here.

Maybe in practice that's the case anyway and we just don't change these things very often but would be interested in your thoughts.

@dnkrj
Copy link
Author

dnkrj commented Sep 5, 2023

Linting actions for Whitehall will be defined in the Whitehall repo

@dnkrj dnkrj closed this Sep 5, 2023
@sengi sengi deleted the 0000-lint branch December 8, 2023 16:53
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.

2 participants