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

[ci/bazel][1] bazelize some unit-tests #35030

Merged
merged 4 commits into from
May 4, 2023
Merged

[ci/bazel][1] bazelize some unit-tests #35030

merged 4 commits into from
May 4, 2023

Conversation

can-anyscale
Copy link
Collaborator

Why are these changes needed?

Bazelize some of the tests I created. Without this it does not run on CI at all.

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • Testing Strategy
    • Unit tests

Signed-off-by: Cuong Nguyen <[email protected]>
Signed-off-by: Cuong Nguyen <[email protected]>
@can-anyscale can-anyscale requested a review from a team as a code owner May 4, 2023 05:07
@can-anyscale can-anyscale requested a review from aslonnie May 4, 2023 05:08
Signed-off-by: Cuong Nguyen <[email protected]>
Copy link
Contributor

@krfricke krfricke left a comment

Choose a reason for hiding this comment

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

Thanks! We do have a script somewhere that checks if all test_ files are tracked in Bazel scripts (for the regular CI) - maybe we can activate that for the release package as well?

@krfricke krfricke merged commit 843de0f into master May 4, 2023
@krfricke krfricke deleted the can-bazel branch May 4, 2023 21:21
@aslonnie
Copy link
Collaborator

aslonnie commented May 4, 2023

ugh... why is this merged..

@aslonnie
Copy link
Collaborator

aslonnie commented May 4, 2023

for context, was trying to get #34689 merged first.

@can-anyscale
Copy link
Collaborator Author

There were some communications of not merge on the [ci/bazel][2] PRs but not this. My bad, should have move it to draft

@aslonnie
Copy link
Collaborator

aslonnie commented May 4, 2023

it was not on "MergeReady" state (which is why I left it unreviewed)..

anyways, I can resolve the conflict. all good.

architkulkarni pushed a commit to architkulkarni/ray that referenced this pull request May 16, 2023
Bazelize some of the tests I created. Without this it does not run on CI at all.

Signed-off-by: Cuong Nguyen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants