-
Notifications
You must be signed in to change notification settings - Fork 9
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
Release 2.1.0 into develop #305
Conversation
😡 (at least I'm glad I'm testing this before starting working on bigger changes…) [EDIT] Fixed via fd1a15d |
Ensuring that all steps install not only the gems but also the brew dependencies needed to install those gems (esp. rmagick)
20ab46b
to
fd1a15d
Compare
#!/bin/bash -eu | ||
|
||
echo "--- :beer: Installing Dependencies" | ||
brew bundle --file .buildkite/brewfile |
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.
Note that this takes a bit of time to run on CI – especially since it's done by each separate step, as each one runs in a separate VM, so each step installs brew dependencies individually – and would benefit some caching.
That being said, caching brew installs, unlike other dependencies like gems or pods, has proven quite tricky and not always trivial, especially since it installs stuff system-wide, and not always all in a single location or folder we could cache. My past experience at trying to cache brew installs have often led to quirks, issues and instabilities (depending on the formulae installed and what they install and where on the system), which is why I didn't spend time on this yet.
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 had trouble with that in the past, too.
I don't think it's a big issue for the moment, albeit it's admittedly "wasteful" to do that every time.
I wonder if it would be too much of a stretch to provision our VMs with those tools, since all the steps here run on those?
Because the script is always called from the root of the repo, via `.buildkite/gem-push`, as opposed to us `cd`-ing into the `.buildkite` dir to run the `gem-push.sh` script, so the `$PWD` is still the repo root in that context.
2.1.0 has now been released and pushed to RubyGems 🎉 . Changes have already been merged into |
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.
Admittedly I still don't have much familiarity with the release-toolkit, so I am always reluctant to review these PRs. However, since the tag is already created and everything in this PR seems reasonable, I am going to approve it and leave it to @AliSoftware to decide whether to merge it or wait for another review.
#!/bin/bash -eu | ||
|
||
echo "--- :beer: Installing Dependencies" | ||
brew bundle --file .buildkite/brewfile |
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 had trouble with that in the past, too.
I don't think it's a big issue for the moment, albeit it's admittedly "wasteful" to do that every time.
I wonder if it would be too much of a stretch to provision our VMs with those tools, since all the steps here run on those?
New version 2.1.0
There's not much new compared to
2.0.0
but it still felt like a good idea to do a new release so soon because:gem push
job on Buildkite early, and fix any issue sooner rather than later if there's any, instead of encountering them while I'll be working on bigger changes.[EDIT] In fact after opening this PR, the CI failed because we forgot to install some dependencies. Not sure why it worked in #299 but not here now that I look at the pipeline 🤔 . So I took the occasion to fix CI via fd1a15d too, hence the extra changes in this diff.