Skip to content
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

Buildkite auto jobs and check dhall #4823

Merged
merged 16 commits into from
May 5, 2020
Merged

Conversation

bkase
Copy link
Member

@bkase bkase commented May 2, 2020

Included here:

  1. Auto-gen jobs config based on jobs directory
  2. Git fetch origin before computing file diffs (necessary so agents have up-to-date view)
  3. New job for checking all Dhall entrypoints (the jobs, prepare, and monorepo-triage)

Tested by manually running the CI job:

Screen Shot 2020-05-02 at 5 52 46 PM

Epic: #4762

@bkase bkase requested a review from a team as a code owner May 2, 2020 21:53
@@ -0,0 +1,5 @@
# Builds all dhall entrypoints
check:
dhall <<< './src/Prepare.dhall'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should handle the stderr of this to identify which file failed (hint: writing a check-dhall.sh would be helpful here). At minimum, can you pipe the stdout of this to /dev/null?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I like the feedback in stderr and stdout during the dev loop (anecdotally) even if it succeeds, it's nice to be able to eyeball all the yaml. Is this okay?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can leave it for now, but I would still like to see the explicit call out of which file failed like I mentioned. Otherwise you don't know what file failed from the output, do you?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It short circuits on failure and you see something like this:

Screen Shot 2020-05-05 at 2 11 00 PM

The error messages are very clear 👍

check:
dhall <<< './src/Prepare.dhall'
dhall <<< './src/Monorepo.dhall'
ls src/jobs | xargs -I{} bash -c 'dhall <<< ./src/jobs/{}/Pipeline.dhall'
Copy link
Member

@nholland94 nholland94 May 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest: for f in ./src/lib/**/Pipeline.dhall; do dhall --file "$f"; done
Xargs is cool, but for wildcarding, this is easier to read and maintain. If you are married to xargs, still prefer the following due to X-platform compat: echo ./src/jobs/**/Pipeline.dhall | xargs -n1 -I{} bash -c 'dhall --file "{}"'

@bkase bkase merged commit 39a9737 into develop May 5, 2020
@bkase bkase deleted the feature/buildkite-auto-jobs branch May 8, 2020 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants