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

build(Makefile): make python3-config protable #177

Merged
merged 5 commits into from
Dec 12, 2022

Conversation

tai271828
Copy link
Contributor

When using python virtual environment, python-config or python3-config are maybe not included in PATH, and thus the command may return incorrect information of python configuration.

When using python virtual environment, python-config or python3-config
are maybe not included in PATH, and thus the command may return
incorrect information of python configuration.
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
@yungyuc yungyuc added the enhancement New feature or request label Dec 10, 2022
@yungyuc
Copy link
Member

yungyuc commented Dec 10, 2022

I would encourage you to make sure CI passes in your fork before filing a PR.

We should not allow any possibility of running Python 2.
@tai271828
Copy link
Contributor Author

I would encourage you to make sure CI passes in your fork before filing a PR.

This is a good idea. I will do that next time.

By using cmake native helpers, e.g. get_filename_component, we can make
sure the job able to run across platforms.
using make functions while we can since it runs faster than shell
@tai271828
Copy link
Contributor Author

Updated the pull request according to the above comments and suggestions. Ready to review again. Thank you!

Copy link
Member

@yungyuc yungyuc left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks a lot.

@yungyuc yungyuc merged commit 8b5f76b into solvcon:master Dec 12, 2022
@tai271828 tai271828 deleted the pr-protable-python-config branch December 12, 2022 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants