Skip to content

Updated CI structure to increase coverage #865

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

Merged
merged 2 commits into from
Aug 16, 2021
Merged

Conversation

UebelAndre
Copy link
Collaborator

@UebelAndre UebelAndre commented Jul 30, 2021

This change is related to #830

@google-cla google-cla bot added the cla: yes label Jul 30, 2021
@UebelAndre UebelAndre force-pushed the ci branch 18 times, most recently from 53de1e9 to d041bef Compare July 30, 2021 22:46
@UebelAndre
Copy link
Collaborator Author

@UebelAndre UebelAndre force-pushed the ci branch 2 times, most recently from 8d064c2 to 6efb3f7 Compare August 10, 2021 12:43
@UebelAndre UebelAndre marked this pull request as ready for review August 10, 2021 12:47
@UebelAndre UebelAndre force-pushed the ci branch 2 times, most recently from 0ba01ae to b9db62d Compare August 12, 2021 16:41
Copy link
Collaborator

@illicitonion illicitonion left a comment

Choose a reason for hiding this comment

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

This looks reasonable to me - I'd be tempted (though it's a big step) to maybe write a tiny python script to generate these, rather than writing them all manually, given at this stage we're building up a bit of a combinatorial explosion...

@UebelAndre
Copy link
Collaborator Author

This looks reasonable to me - I'd be tempted (though it's a big step) to maybe write a tiny python script to generate these, rather than writing them all manually, given at this stage we're building up a bit of a combinatorial explosion...

Yeah, but I don't feel like the jobs change so frequently that it's worth that kind of effort. If the issue is that the yaml is not that clear anymore, I'd ask for some pointers in yaml-fu before trying to write some generation tool 😄. Do you have any suggestions on that front?

@illicitonion
Copy link
Collaborator

This looks reasonable to me - I'd be tempted (though it's a big step) to maybe write a tiny python script to generate these, rather than writing them all manually, given at this stage we're building up a bit of a combinatorial explosion...

Yeah, but I don't feel like the jobs change so frequently that it's worth that kind of effort.

Fair! Now definitely doesn't have to be The Time.

If the issue is that the yaml is not that clear anymore, I'd ask for some pointers in yaml-fu before trying to write some generation tool 😄. Do you have any suggestions on that front?

In my experience, it's simply not possible to make yaml more readable :)

@UebelAndre
Copy link
Collaborator Author

@hlopko If there are no objections from you, I'd like to get this merged. I feel in general it's a good improvement

@hlopko
Copy link
Member

hlopko commented Aug 16, 2021

I stand aside, feel free to merge :)

@UebelAndre UebelAndre merged commit af2f908 into bazelbuild:main Aug 16, 2021
@UebelAndre UebelAndre deleted the ci branch August 16, 2021 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants