-
Notifications
You must be signed in to change notification settings - Fork 11
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
Set up CI #88
Conversation
Specifically we need the sortByReference type fix from v3.8.0: https://github.com/amzn/style-dictionary/blob/e8aea2f68404a2c4841ee5eb55525e837f34e326/CHANGELOG.md#380-2023-04-25
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.
LGTM, just a comment about a potential minor issue.
export const cpdFontBodyXsRegular = "400 0.688rem/1.5 Inter"; | ||
export const cpdFontBodyXsMedium = "500 0.688rem/1.5 Inter"; | ||
export const cpdFontBodyXsSemibold = "600 0.688rem/1.5 Inter"; | ||
export const cpdFontBodySmRegular = "400 0.813rem/1.5 Inter"; | ||
export const cpdFontBodySmMedium = "500 0.813rem/1.5 Inter"; | ||
export const cpdFontBodySmSemibold = "600 0.813rem/1.5 Inter"; | ||
export const cpdFontBodyMdRegular = "400 0.938rem/1.5 Inter"; | ||
export const cpdFontBodyMdMedium = "500 0.938rem/1.5 Inter"; | ||
export const cpdFontBodyMdSemibold = "600 0.938rem/1.5 Inter"; | ||
export const cpdFontBodyLgRegular = "400 1.063rem/1.5 Inter"; | ||
export const cpdFontBodyLgMedium = "500 1.063rem/1.5 Inter"; | ||
export const cpdFontBodyLgSemibold = "600 1.063rem/1.5 Inter"; |
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.
These actually change the size, even if not by much. Is that ok?
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.
Oh, this should actually go into #87 😅
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.
Ah, I missed this before I merged the other PR. I'll try to research why Style Dictionary is rounding the values, and whether it messes with anything.
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.
(as in, I'll do that in another PR now that it's already too late…)
I changed the base to the |
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!
Let's start using GitHub Actions to check formatting/lints and ensure that all generated files are up to date.
Based on #87 - please only review the later commits