-
Notifications
You must be signed in to change notification settings - Fork 4
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
Conversation
There was a problem hiding this 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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
Not seeing any major differences so far.
Before | After |
---|---|
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
.github/workflows/formatting.yml
Outdated
- name: Install dependencies | ||
run: npm ci | ||
- name: Run prettier | ||
run: 'npm run prettier:js' |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
@heymatthenry thanks for the suggestions. Changes were minor, so I've added them and updated the PR description to include the new info. |
There was a problem hiding this 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 😎
Closes #33.
A new CI test that runs prettier on push.
Major changes
.prettierrc
and.prettierignore
as best practice.[npm runprettier:js]
and MDX[npm run prettier:md]
files. Formatting fixes included.How to test
npm run prettier:js && npm run prettier:md
.${ this.href }
.{ Readme }
.npm run prettier:js:fix && npm run prettier:md:fix
to fix issues.npm run prettier:js:fix && npm run prettier:md:fix
.