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

main: unify output for resolve/db commands, make it parseable #462

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

main: unify output for resolve/db commands, make it parseable #462

wants to merge 1 commit into from

Conversation

proger
Copy link

@proger proger commented Jun 13, 2016

I also squeezed a tiny cleanup in form of a resolve_and_print function.

I'm not sure if the tool output is supposed to be stable, but this PR makes resolve and db look awk'able (and print the installer by default to save a roundtrip).

installer = installer_context.get_installer(rule_installer)
resolved = installer.resolve(rule)
print("%s:%s:%s"%(rosdep_name, ",".join([str(r) for r in resolved]), rule_installer))
except ResolutionError as e:
Copy link
Author

Choose a reason for hiding this comment

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

not sure if both KeyError and ResolutionError handlers are necessary

@wjwwood
Copy link
Contributor

wjwwood commented Jul 28, 2017

Sorry, I know this issue is over a year old.

I don't think we can do this. I do know of at least a few places the output of db is being used. Perhaps we could introduce an optional --output-version= option. Where by default it would be something like plain_text-v1. And this could be plain_text-v2 or something like that. We could also have something like json-vX in the future which dumped it as some kind of JSON structure. I'm drawing inspiration from brew info --json=... on this idea.

Unfortunately, as-is I don't feel comfortable merging this as it departs from the existing out and rosdep is not tied to ROS versions so it would affect all versions, past and present.

Copy link
Contributor

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

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

Needs to preserve old output, either as the default or somehow able to opt-into the old behavior.

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

Successfully merging this pull request may close these issues.

2 participants