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

Change some deps* function to public. #65

Closed
wants to merge 2 commits into from

Conversation

130s
Copy link
Member

@130s 130s commented Dec 30, 2016

Rosstackage::stackage contains useful info, or at least it's more informative than just the package names that some public deps* functions provide as a result of them running. In some applications, being able to access stackage objects directly makes code much cleaner; otherwise we will have to manually obtain stackages by passing package names (, which is still feasible though).

Is there any reason to not public-ize these functions? I thought it could be because we may not want to expose stackage publicly to the library users, but then I don't know why so.

@130s 130s changed the title J/move somefuncs public Change some deps* function to public. Dec 30, 2016
@dirk-thomas
Copy link
Member

This can be considered for a future release of ROS. But since it changes ABI I don't think it should be merged into any of the existing ROS distributions.

@dirk-thomas
Copy link
Member

I just created the new lunar-devel branch and cherry picked your patch: 4b912a5 and d1858bf

Thank you for the contribution.

@130s 130s deleted the j/move_somefuncs_public branch February 23, 2017 00:35
130s added a commit to 130s/rospack that referenced this pull request Feb 25, 2017
Suggested code in ros#65, which changes existing methods signature, breaks ABI compatibility.

Instead, this PR adds new methods that internally call existing methods, to keep ABI compatible.
130s added a commit to 130s/rospack that referenced this pull request Feb 25, 2017
Suggested code in ros#65, which changes existing functions signature, breaks ABI compatibility.

Instead, this PR adds new functions that internally call existing functions, to keep ABI compatible.
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.

2 participants