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

Remove exporting symbols #22623

Merged
merged 3 commits into from
Feb 28, 2022
Merged

Remove exporting symbols #22623

merged 3 commits into from
Feb 28, 2022

Conversation

ashione
Copy link
Member

@ashione ashione commented Feb 24, 2022

Why are these changes needed?

To hidden symbols of thirdparty library, this pull request reuses internal namespace that can be imported by any external native projects without side effects.

Besides, we suggest all of contributors to make sure it'd better use thirdparty library in ray scopes/namspaces and only ray::internal should be exported.

More details in #22526

Mobius has applied this change in ray-project/mobius#28.

Related issue number

Checks

  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@fishbone
Copy link
Contributor

Could you please fill the description in the PR?

@rkooo567
Copy link
Contributor

Thanks for the change! Can you update the description?

Copy link
Contributor

@scv119 scv119 left a comment

Choose a reason for hiding this comment

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

thanks for making the change!

@jovany-wang
Copy link
Contributor

rllib:learning_tests_transformed_actions_pendulum_sac failure is unrelated, which also fails on master.

@jovany-wang jovany-wang merged commit 94caac8 into master Feb 28, 2022
@jovany-wang jovany-wang deleted the remove-exporting-symbols branch February 28, 2022 01:41
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.

6 participants