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

USWDS-Next - CI: Create a GH action to test code formatting #32

Merged
merged 7 commits into from
Jul 22, 2024

Conversation

mejiaj
Copy link
Collaborator

@mejiaj mejiaj commented Jul 10, 2024

Closes #33.

A new CI test that runs prettier on push.

Before After
image image

Major changes

  • GitHub actions npm caching: For performance, I've added global npm caching to GitHub actions.
  • Added prettier config: Added .prettierrc and .prettierignore as best practice.
  • Added formatting tests: New tests for testing JS [npm runprettier:js] and MDX [npm run prettier:md] files. Formatting fixes included.

How to test

  1. Run npm run prettier:js && npm run prettier:md.
  2. There should be zero errors.
  3. Modify a JS file to add an error like ${ this.href }.
  4. You should see a code style issue.
  5. Modify an MDX file to add an error like { Readme }.
  6. You should see a code style issue.
  7. Run npm run prettier:js:fix && npm run prettier:md:fix to fix issues.
  8. Styles should match after re-running npm run prettier:js:fix && npm run prettier:md:fix.

@mejiaj mejiaj marked this pull request as ready for review July 10, 2024 16:56
@mejiaj mejiaj requested a review from heymatthenry July 10, 2024 16:57
Copy link
Collaborator

@heymatthenry heymatthenry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this! Automate all the things 🤖

with:
node-version: '18.x'
- name: Install dependencies
run: npm ci
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought: At some point we may need to move tasks around between actions to cut down on the number of times we're running npm ci in CI

Copy link
Collaborator Author

@mejiaj mejiaj Jul 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a great thought.

The setup-node action supports caching. Added in ef95346.

You should now see a caches link in actions.

image

Not seeing any major differences so far.

Before After
image image

https://github.com/uswds/uswds-next/actions/caches


Tip

Monorepo dependencies can be cached too. Example below.

Monorepo dependency cache

Details

steps:
- uses: actions/setup-node@v4
  with:
    node-version: '18.x'
    cache: 'npm'
    cache-dependency-path: '**/package-lock.json'
- run: npm ci
- run: npm test

Source: https://github.com/actions/setup-node?tab=readme-ov-file#caching-global-packages-data

- name: Install dependencies
run: npm ci
- name: Run prettier
run: 'npm run prettier:js'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (non-blocking): Doesn't have to be in this PR but it might be a good idea to add a minimal prettier config even if we're just using defaults. Some editors need a config in order to run prettier, and it's just helpful to have at the project root to signal intent.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed with clear intent.

Added .prettierignore and .prettierrc in 6342c0d.

@mejiaj mejiaj requested a review from heymatthenry July 11, 2024 14:31
@mejiaj
Copy link
Collaborator Author

mejiaj commented Jul 11, 2024

@heymatthenry thanks for the suggestions.

Changes were minor, so I've added them and updated the PR description to include the new info.

Copy link
Collaborator

@heymatthenry heymatthenry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this! Merging it in so I get all the 🤖 goodness while I dig into package configs 😎

@heymatthenry heymatthenry merged commit 1cc3b3a into develop Jul 22, 2024
5 checks passed
@mejiaj mejiaj deleted the jm/gh-action-formatting branch July 22, 2024 16:56
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.

USWDS-Next - CI: Add a check for code formatting
2 participants