-
Notifications
You must be signed in to change notification settings - Fork 828
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
Avoid duplicate build folder in @primer/octicons #1001
Conversation
|
@@ -17,7 +17,7 @@ | |||
"url": "https://github.com/primer/octicons/issues" | |||
}, | |||
"scripts": { | |||
"build": "\\cp -r ../build/ ./build && \\cp index.scss ./build/build.css", | |||
"build": "\\cp -r ../build/. ./build && \\cp index.scss ./build/build.css", |
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.
Hi! This pull request has been marked as stale because it has been open with no activity for 60 days. You can comment on the pull request or remove the stale label to keep it open. If you do nothing, this pull request will be closed in 7 days. |
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.
Nice, thank you!
I'm not sure what's going on with these build failures, but I assume it's because this PR was created from a fork. I just pushed a copy up to origin to see if that fixes things: #1010 |
Merged in #1010 |
In my local tests, running
yarn build
inlib/octicons_node
generatesbuild/build/svg
but after this change it generatesbuild/svg
, so it should fix the issue.However the build is a bit of spaghetti between workflows, docker entrypoints.js and multiple package.json, so it's not entirely clear if further work is required (probably not)