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

end --only-packages with -- #10

Merged
merged 1 commit into from
Nov 8, 2016
Merged

end --only-packages with -- #10

merged 1 commit into from
Nov 8, 2016

Conversation

dirk-thomas
Copy link
Member

Follow up of ament/ament_tools#114.

@dirk-thomas dirk-thomas added the in review Waiting for review (Kanban column) label Nov 8, 2016
@dirk-thomas dirk-thomas self-assigned this Nov 8, 2016
@dirk-thomas dirk-thomas merged commit 1195ba7 into master Nov 8, 2016
@dirk-thomas dirk-thomas removed the in review Waiting for review (Kanban column) label Nov 8, 2016
@dirk-thomas dirk-thomas deleted the only_packages branch November 8, 2016 16:00
@dhood
Copy link
Member

dhood commented Nov 8, 2016

I think it's better to move the sourcespace to the front of the argument list since the -- delimiter will not work if any other optionals are added.

@dhood
Copy link
Member

dhood commented Nov 8, 2016

since this has been merged we'll just have to keep this in mind in the future.

@dirk-thomas
Copy link
Member Author

I don't think a trailing -- makes the command fail. Some of my local bash aliases always do that and work fine.

@dhood
Copy link
Member

dhood commented Nov 8, 2016

If we want to package in isolation, with how it's currently setup in line 53, I expect the job to fail:

$ ./src/ament/ament_tools/scripts/ament.py build --only-packages ament_tools -- ./src --isolated
usage: ament.py [-h]
                [build | build_pkg | list_packages | package_name | package_version | test | test_pkg | test_results | uninstall | uninstall_pkg]
                ...
ament.py: error: unrecognized arguments: --isolated

that's why I recommend moving the positional arguments to the front e.g.

$ ./src/ament/ament_tools/scripts/ament.py build ./src --only-packages ament_tools --isolated

@dirk-thomas
Copy link
Member Author

Sounds good to me: #11

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.

3 participants