Skip to content

Conversation

@castastrophe
Copy link
Contributor

@castastrophe castastrophe commented May 30, 2025

Description

This migrates linting from CircleCI to GitHub Actions.

Motivation and context

The current linting in CI is more-or-less boolean (did it pass or not) but requires pulling down the branch locally and linting to fix things. This update moves our linting work into a GitHub Action that uses ReviewDog to lint and report back to the pull request what the linter identified.


Author's checklist

  • I have read the CONTRIBUTING and PULL_REQUESTS documents.
  • I have reviewed at the Accessibility Practices for this feature, see: Aria Practices

Reviewer's checklist

  • Includes a Github Issue with appropriate flag or Jira ticket number without a link
  • Includes thoughtfully written changeset if changes suggested include patch, minor, or major features
  • Automated tests cover all use cases and follow best practices for writing
  • Validated on all supported browsers
  • All VRTs are approved before the author can update Golden Hash

Manual review test cases

@changeset-bot
Copy link

changeset-bot bot commented May 30, 2025

⚠️ No Changeset found

Latest commit: 3fca8ae

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions
Copy link
Contributor

Branch preview

Review the following VRT differences

When a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:

If the changes are expected, update the current_golden_images_cache hash in the circleci config to accept the new images. Instructions are included in that file.
If the changes are unexpected, you can investigate the cause of the differences and update the code accordingly.

@castastrophe castastrophe force-pushed the castastrophe/chore-ci-move-linting branch from ea8edec to 32f760f Compare May 30, 2025 15:15
@github-actions
Copy link
Contributor

Tachometer results

Currently, no packages are changed by this PR...

@castastrophe castastrophe force-pushed the castastrophe/chore-ci-move-linting branch from 32f760f to 6ae9cc7 Compare May 30, 2025 15:19
key: v2-golden-images-{{ .Branch }}-<< parameters.regression_system >>-<< parameters.regression_color >>-<< parameters.regression_scale >>-<< parameters.regression_dir >>-{{ epoch }}

jobs:
commitlint:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is handled by pre-commit so we don't need to duplicate this here

- store_test_results:
path: /root/project/results/

lint:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved linting into a GitHub action


concurrency:
group: '${{ github.workflow }} @ ${{ github.event.pull_request.head.label || github.head_ref || github.ref }}'
cancel-in-progress: true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This ensures we don't end up with a queue for linting a PR

with:
files_yaml: |
styles:
- '*.css'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I kept these pretty open-ended but we can refine this list as we go

run: yarn install --immutable

- name: Lint styles
if: ${{ needs.changed_files.outputs.styles_added_files != '' || needs.changed_files.outputs.styles_modified_files != '' }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This ensures the task is only run if there are files to lint

tach-results.*
.gitconfig
.netlify/plugins
.env
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not suuuuuper related but we don't want to commit an env file yet.

},
"overrides": [
{
"files": [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This might be something we missed during cutover which is that we should probably start linting these files now that they're manually written rather than pre-processed.

Copy link
Contributor

Choose a reason for hiding this comment

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

makes sense

@castastrophe castastrophe force-pushed the castastrophe/chore-ci-move-linting branch 2 times, most recently from ef02e9d to 05cc5d5 Compare May 30, 2025 15:36
Comment on lines 1 to 3
{
"private": true,
"name": "@adobe/spectrum-web-components",
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ [eslint] <jsonc/sort-keys> reported by reviewdog 🐶
Expected object keys to be in specified order. 'name' should be before 'private'.

Suggested change
{
"private": true,
"name": "@adobe/spectrum-web-components",
{
"name": "@adobe/spectrum-web-components",
"private": true,

@castastrophe castastrophe force-pushed the castastrophe/chore-ci-move-linting branch from 05cc5d5 to cb1d411 Compare May 30, 2025 15:40
@castastrophe castastrophe marked this pull request as ready for review May 30, 2025 15:40
@castastrophe castastrophe requested a review from a team as a code owner May 30, 2025 15:40
@castastrophe castastrophe self-assigned this May 30, 2025
@castastrophe castastrophe added Component: Tooling Issue or PR dealing with scripts, workflows, automation, etc. Component prefix is for Jira Status: Ready for review PR ready for review or re-review. labels May 30, 2025
Copy link
Contributor

@nikkimk nikkimk left a comment

Choose a reason for hiding this comment

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

Tiny nit: can we associate a ticket number with this? Otherwise, let girls train monsters (LGTM)!

@castastrophe castastrophe force-pushed the castastrophe/chore-ci-move-linting branch 2 times, most recently from dca4492 to bb1c28f Compare June 10, 2025 22:08
@castastrophe castastrophe force-pushed the castastrophe/chore-ci-move-linting branch from bb1c28f to 3fca8ae Compare June 11, 2025 15:14
@castastrophe castastrophe enabled auto-merge (squash) June 11, 2025 15:14
@castastrophe castastrophe merged commit feb6b13 into main Jun 11, 2025
23 of 25 checks passed
@castastrophe castastrophe deleted the castastrophe/chore-ci-move-linting branch June 11, 2025 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: Tooling Issue or PR dealing with scripts, workflows, automation, etc. Component prefix is for Jira Status: Ready for review PR ready for review or re-review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants