-
Notifications
You must be signed in to change notification settings - Fork 5
pull out common pieces of yaml in the buildkite pipeline #976
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
base: master
Are you sure you want to change the base?
Conversation
- &aws_sm_plugin | ||
seek-oss/aws-sm#v2.3.1: | ||
region: us-east-2 | ||
env: | ||
BUILDEVENT_APIKEY: honeycomb-api-key | ||
BUILDEVENT_BUILDKITE_API_TOKEN: buildkite-api-token-honeycomb-build-events | ||
- &buildevents_plugin | ||
replayio/buildevents#adb8a05: ~ |
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.
Ideally I would be able to express this as:
- &buildevents_plugins
- seek-oss/aws-sm#v2.3.1:
region: us-east-2
env:
BUILDEVENT_APIKEY: honeycomb-api-key
BUILDEVENT_BUILDKITE_API_TOKEN: buildkite-api-token-honeycomb-build-events
- replayio/buildevents#adb8a05: ~
and then reference the anchored list below like:
- &linux_plugins
plugins:
- thedyrt/skip-checkout#v0.1.1:
cd: /home/ubuntu/chromium/src
<<: *buildevents_plugins
but alas, that isn't valid yaml. instead each plugin needs its own anchor and those get mentioned individually in the os plugins lists.
BUILDEVENT_APIKEY: honeycomb-api-key | ||
BUILDEVENT_BUILDKITE_API_TOKEN: buildkite-api-token-honeycomb-build-events | ||
- replayio/buildevents#adb8a05: ~ | ||
- &buckle_build |
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.
q: is there a reason not to use buckle everywhere? that would make it so we can reuse the same build command sequence across linux/macos.
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.
the buck2/buckle build anchors are identical except for two lines. in fact except for two words. This is another place where yaml list merging (and string concatenation) would help a bunch. I didn't really want to create anchors for each shared command here, like I did up in plugins
.
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.
Eventually we'll be able to use buckle everywhere, but there's currently a bug in the Windows version benbrittain/buckle#21
the diff here is pretty hard to read. probably easier to look at the resulting file. |
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!
4bbbe15
to
fb2a388
Compare
rebased off master ☝️ |
fb2a388
to
10f2b66
Compare
and one last rebase to make sure nothing's worse off than before, before I land. |
This was initially motivated by what I saw as a bunch of needless repetition in
plugins
due to the buildevents work in #943, but I found other things that could be pulled out as well.Found a pretty big 🤦 in that yaml can't seem to deal with anchors to and merging references of lists. This causes a bunch of pain in both the
commands
andplugins
anchors that I'll call out inline, and it means we can't expressplugins
/commands
in the steps using the syntax thatagents
/env
uses.