-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: build react/vue/elements icons #4
Conversation
2ee4d3d
to
c958b28
Compare
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.
I have some comments that made me think that we should maybe handle publishing/automated releases in a separate PR. This PR is already a bit too big imo
On that note, I think we should aim at having smaller PRs in the future. Especially when we squash-merge PRs that are not going intomain
. The commit messages then don't cover all that has been changed well enough in my opinion. It's also easier to get good reviews when there's less to look through in the changed files 😊
package.json
Outdated
"dev": "vite", | ||
"vite:build": "vite build", | ||
"preview": "pnpm run build && vite build && vite preview" | ||
"preview": "pnpm run build && vite build && vite preview", |
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.
Just to align with other examples :)
"preview": "pnpm run build && vite build && vite preview", | |
"preview": "pnpm build && vite build && vite preview", |
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.
Fixed in da0b941. Here I also renamed the command that shows a preview of all icons to pnpm dev
. This way all our repos have a similar command to see the output of that package locally. Tested that the output is the same when using pnpm build && vite
as in pnpm build && vite build && vite preview
.
I opened a PR just for updating dependencies: #6 so we can include fewer changes in this PR. |
61a25df
to
1166627
Compare
1166627
to
0f02988
Compare
Align with other warp-ds repos on how we can check the output in the form of a running 'docs page.
0f02988
to
da0b941
Compare
Sorry for the mess. I wanted to move the logic for automating releases out to make the changes easier to review. There's a draft PR for handling automated releases here: #7. |
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 🎉
🎉 This PR is included in version 1.0.0-alpha.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Background: https://nmp-jira.atlassian.net/jira/software/projects/WARP/boards/15?selectedIssue=WARP-224
Publishing to NPM and EIK we will be handled in a separate PR.