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

[Windows][noetic] a follow up fix to the generated executable change. #1063

Merged
merged 2 commits into from
Feb 8, 2020

Conversation

seanyen
Copy link
Contributor

@seanyen seanyen commented Feb 7, 2020

After more validation to my earlier changes of "Generate executables without extension name", I overlooked a required change in python_win32_wrapper.cpp.in side. Please excuse me causing any inconvenience.

@seanyen
Copy link
Contributor Author

seanyen commented Feb 7, 2020

@dirk-thomas This is ready for review and merge. Thanks!

@dirk-thomas
Copy link
Member

dirk-thomas commented Feb 7, 2020

Why should the code need to guess? The script name is known at configure time - if it does or does not contain a .py extension. How about embedding that knowledge into the .cppfile?

@seanyen
Copy link
Contributor Author

seanyen commented Feb 7, 2020

@dirk-thomas Sure, I changed the template .cpp to refer to @ARG_SCRIPT_NAME@ for the script name.

@dirk-thomas dirk-thomas added the bug label Feb 8, 2020
@dirk-thomas
Copy link
Member

Thanks for the patch.

@dirk-thomas dirk-thomas merged commit 89708cc into ros:noetic-devel Feb 8, 2020
dirk-thomas pushed a commit that referenced this pull request Feb 8, 2020
…#1063)

* a follow up fix to the generated exectuable change.

* address the feedback.
@dirk-thomas
Copy link
Member

Also cherry-picked to kinetic-devel: ebaa3c5.

@seanyen
Copy link
Contributor Author

seanyen commented Feb 8, 2020

@dirk-thomas Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants