-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
build: cache ui dependencies in CI #2713
Conversation
|
||
CMD ["/bin/bash"] | ||
EOF | ||
cp "$(dirname $0)"/../ui/{bower,npm-shrinkwrap,package,tsd}.json "$(dirname $0)"/../ui/Makefile "$(dirname $0)" |
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.
are we going to frequently update these? Just asking since any change here requires a manual tag change in circle-deps
(which isn't nicely automated b/c of likely caching issues).
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 expect these will be updated a few times in the near future at least as I start building out the site.
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.
ok, then that solution here looks good.
whoops, misread that as "few times a year". Well, we gotta start somehow. It's only if they changed multiple times a week forever that this would be an issue.
LGTM |
Reworked this so the caching is in circle-ci instead of in the builder image itself. We'll invoke |
|
||
ENV SKIP_BOOTSTRAP=1 | ||
|
||
CMD ["/bin/bash"] |
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.
You can move this back to builder.sh
, right? Doesn't really matter I suppose.
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.
Yeah, doesn't really matter. Slightly less surprising this way, I think.
LGTM. This is much better. |
@petermattis PTAL, had to add a commit because node stuff appears to be owned by root which fails on the cache pruning bit in builder.sh |
LGTM |
Ran this build a second time after getting a green; confirmed the cache was used and the various dependency fetches were no-ops. |
build: cache ui dependencies in CI
Partially reverts cockroachdb#2713 with a comment explaining why it should be the way it was. Attempt to fix the use of `circle-local.sh` in a dev environment but don't get all the way there.
Closes #2705.
More useful now that we have npm, bower, and tsd.