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

Add AMENT_PREFIX_PATH environment hook #7

Merged
merged 2 commits into from
Jan 6, 2022

Conversation

jacobperron
Copy link
Contributor

This is a quick fix to a problem described in ros2-java/ament_java#9

TL;DR
Gradle projects are not being added to the AMENT_PREFIX_PATH; this PR let's colcon handle it.

@esteve
Copy link
Collaborator

esteve commented Aug 11, 2020

@jacobperron thanks! Sorry I forgot about this one. I only have one question, but overall it looks good.

Signed-off-by: Jacob Perron <[email protected]>
@jacobperron
Copy link
Contributor Author

I think this PR is ready. @esteve @ivanpauno, if either of you give me an approval I'll merge it.

@YasuChiba
Copy link
Contributor

@esteve @ivanpauno

Hi,
Thank you for maintaining ros2-java and related projects!
I think this PR is important to everyone who is using ros2java. (include me :) )
If it's possible, can you merge this PR?

@@ -23,6 +25,21 @@ def add_arguments(self, *, parser): # noqa: D102
'--ament-gradle-task',
help='Run a specific task instead of the default task')

async def build(
self, *, additional_hooks=[], skip_hook_creation=False
): # noqa: D102
Copy link
Collaborator

Choose a reason for hiding this comment

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

question: what is this noqa for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"Missing docstring in public method"

@jacobperron jacobperron merged commit 56558b9 into master Jan 6, 2022
@delete-merged-branch delete-merged-branch bot deleted the jacob/ament_prefix_path_hook branch January 6, 2022 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants