Skip to content

chore(ci): update compare assets processing #2337

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

Merged
merged 1 commit into from
Dec 5, 2023

Conversation

castastrophe
Copy link
Collaborator

@castastrophe castastrophe commented Dec 4, 2023

Description

A lightweight update to:

  • Minor eslint updates (move to use warn instead of error to prevent the linter from blocking builds)
  • Rename .github/README.md b/c it's using this file for our landing page instead of the root README - https://github.com/adobe/spectrum-css#github-actions-templates-and-bots-oh-my (oops!)
  • Update the file-diff output to a more streamlined comment format with a clear summary of changes to components at the top
  • Comment-in the scoped stylelint files but leave eslint off for CI at the moment
  • Migrate storybook to use project.json for the commands & workflow settings
  • Move scripts in package.json from lerna to nx for parallelization (this will fix the caching errors we get when we alernate running lerna & nx for build tasks)
  • Standardize licensing and authors for embedded packages to match open source standards (Apache-2.0 & Adobe)

How and where has this been tested?

No regressions for the following commands (tested by @mdt2):

  • yarn dev
    • No regression on spinup, css change, or yml change.
  • yarn start
    • No regression on spinup, css change, or story change.
  • yarn build:preview
    • Tested this by deleting .storybook/storybook-static, then ran yarn build:preview which built storybook-static. Then I ran yarn start and confirmed that what was built is working as expected locally.
  • yarn build:components
    • Yep! After cleaning, this rebuilds the dist folders for the components.
  • yarn clean:components
    • Removes components dist

To-do list

  • I have read the contribution guidelines.
  • If my change impacts documentation, I have updated the documentation accordingly.
  • ✨ This pull request is ready to merge. ✨

@castastrophe castastrophe force-pushed the chore-ci-compare-assets branch 6 times, most recently from 8158c71 to a92cf49 Compare December 5, 2023 14:03
Copy link
Contributor

github-actions bot commented Dec 5, 2023

File metrics

Summary

Total size: 3.61 MB*

🎉 No changes detected in any packages

* An ASCII character in UTF-8 is 8 bits or 1 byte.

@castastrophe castastrophe force-pushed the chore-ci-compare-assets branch 5 times, most recently from a3eb7cd to d1538d9 Compare December 5, 2023 15:05
@castastrophe castastrophe requested a review from pfulton December 5, 2023 15:05
@castastrophe castastrophe added the run_vrt For use on PRs looking to kick off VRT label Dec 5, 2023
Copy link
Contributor

github-actions bot commented Dec 5, 2023

🚀 Deployed on https://pr-2337--spectrum-css.netlify.app

@castastrophe castastrophe force-pushed the chore-ci-compare-assets branch 2 times, most recently from b536d71 to b499b29 Compare December 5, 2023 15:34
@castastrophe castastrophe added the size-2 S ~6-18hrs; not hard or time consuming, one or two work days to complete. label Dec 5, 2023
@castastrophe castastrophe force-pushed the chore-ci-compare-assets branch from b499b29 to ef63370 Compare December 5, 2023 16:25
@castastrophe castastrophe requested a review from mdt2 December 5, 2023 16:30
Copy link
Collaborator

@mdt2 mdt2 left a comment

Choose a reason for hiding this comment

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

This is looking really good. The only command I'm not sure about is yarn clean:components. In the console I see The following projects do not have a configuration for any of the provided targets ("clean"), is this expected?

@castastrophe castastrophe force-pushed the chore-ci-compare-assets branch from ef63370 to 699ae34 Compare December 5, 2023 16:55
@castastrophe
Copy link
Collaborator Author

In the console I see The following projects do not have a configuration for any of the provided targets ("clean"), is this expected?

That does not sound right. Let me poke at that. Thanks for checking it!

@castastrophe castastrophe force-pushed the chore-ci-compare-assets branch 2 times, most recently from 6a55b02 to 3cd6641 Compare December 5, 2023 18:40
@castastrophe
Copy link
Collaborator Author

In the console I see The following projects do not have a configuration for any of the provided targets ("clean"), is this expected?

That does not sound right. Let me poke at that. Thanks for checking it!

Alright, this has now been fixed by adding the clean command to each package.json. This will be a relatively short-lived fix since we'll be moving all scripts to be run by project.json & nx.json definitions but it gets us off lerna for running commands since nx & lerna's caches don't play well together.

@castastrophe castastrophe force-pushed the chore-ci-compare-assets branch from d377349 to 217b788 Compare December 5, 2023 19:25
Copy link
Collaborator

@mdt2 mdt2 left a comment

Choose a reason for hiding this comment

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

LGTM! 🎉

Comment on lines +120 to +121
"*.css": [
"stylelint --fix --quiet"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm a little leery of this, just because we had some problems the last time. Let's move forward with this for now, but keep an eye on it for issues, please!

@castastrophe castastrophe merged commit 1f3104c into main Dec 5, 2023
@castastrophe castastrophe deleted the chore-ci-compare-assets branch December 5, 2023 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-review run_vrt For use on PRs looking to kick off VRT size-2 S ~6-18hrs; not hard or time consuming, one or two work days to complete.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants