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

Use PYBIND11_EXPORT instead of visibility hack #8437

Merged
merged 1 commit into from
Nov 21, 2019

Conversation

niboshi
Copy link
Member

@niboshi niboshi commented Nov 13, 2019

Instead of tweaking the default visibility (#5936), this PR applies PYBIND11_EXPORT to relevant classes, as suggested in pybind/pybind11#1272 (comment).

@niboshi niboshi changed the title Use PYBIND11_EXPORT instead of visibility hack Use PYBIND11_EXPORT instead of visibility hack Nov 13, 2019
@niboshi niboshi force-pushed the use-pybind11-export branch 2 times, most recently from 8747035 to 4436240 Compare November 14, 2019 03:00
@hvy
Copy link
Member

hvy commented Nov 15, 2019

Jenkins, test this please.

@chainer-ci
Copy link
Member

Jenkins CI test (for commit d27326e, target branch master) succeeded!

@hvy
Copy link
Member

hvy commented Nov 21, 2019

Thanks, it LGTM. Let me just rerun the CI.

Jenkins, test this please.

@hvy hvy added cat:test Test or CI related. cat:install Build and installation. ChainerX Related to ChainerX. st:test-and-merge State indicating that pull request is approved by a reviewer and can be merged after CI passes. and removed cat:test Test or CI related. labels Nov 21, 2019
@hvy hvy added this to the v7.0.0 milestone Nov 21, 2019
@chainer-ci
Copy link
Member

Jenkins CI test (for commit d27326e, target branch master) succeeded!

@mergify mergify bot merged commit 269c135 into chainer:master Nov 21, 2019
@niboshi niboshi deleted the use-pybind11-export branch November 21, 2019 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cat:install Build and installation. ChainerX Related to ChainerX. st:test-and-merge State indicating that pull request is approved by a reviewer and can be merged after CI passes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants