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.

Sign up for GitHub

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

Jump to bottom

Updated crate_universe examples #676

Merged
merged 3 commits into from
Apr 7, 2021
Merged

Updated crate_universe examples #676

merged 3 commits into from
Apr 7, 2021

Conversation

UebelAndre
Copy link
Collaborator

Simple cleanup of the examples so they can more effectively be run in CI.

@google-cla google-cla bot added the cla: yes label Apr 1, 2021
@UebelAndre UebelAndre changed the title Updated cargo_universe examples Updated crate_universe examples Apr 2, 2021
@UebelAndre
Copy link
Collaborator Author

@illicitonion do you have a quick moment to take a look here?

@UebelAndre UebelAndre mentioned this pull request Apr 2, 2021
Merged
illicitonion
illicitonion approved these changes Apr 7, 2021
View reviewed changes
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.

I feel like this makes the examples a lot less clear, but I can see the value for making CI simpler... Happy to merge for now, but I can see maybe reverting this in the future to make the examples more clear...

illicitonion
illicitonion reviewed Apr 7, 2021
View reviewed changes
.bazelignore
@@ -1,4 +1,2 @@
docs
examples
examples/crate_universe
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this not need to stay?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it's a sub-directory of examples so it should already be ignored. After quite a bit of testing with this change I've noticed no regressions but the documentation for bazelignore is a bit lacking.

@UebelAndre
Copy link
Collaborator Author

I feel like this makes the examples a lot less clear, but I can see the value for making CI simpler... Happy to merge for now, but I can see maybe reverting this in the future to make the examples more clear...

It definitely makes CI easier. The only other option I know of is to have one job per workspace which I would assume has an unfortunate impact on the larger pool of workers. This can definitely be reverted in the future once crate_universe stabilizes a bit and we find this is implementation is undesirable.

@UebelAndre UebelAndre merged commit 9f15a01 into bazelbuild:main Apr 7, 2021
@UebelAndre UebelAndre deleted the examples branch April 7, 2021 15:56
yagehu pushed a commit to yagehu/rules_rust that referenced this pull request Apr 23, 2021
@UebelAndre @illicitonion @yagehu
Updated cargo_universe examples (bazelbuild#676)
8dc8c5c 
Co-authored-by: Daniel Wagner-Hall <dwagnerhall@apple.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@illicitonion illicitonion illicitonion approved these changes

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@UebelAndre @illicitonion