Add push_ros_namespace alias to push-ros-namespace
#250
Conversation
Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
It was pretty straightforward, so I just did it: ros2/ros2_documentation#1703 |
| @@ -33,6 +33,7 @@ | |||
| from rclpy.validate_namespace import validate_namespace | |||
|
|
|||
|
|
|||
| @expose_action('push_ros_namespace') | |||
| @expose_action('push-ros-namespace') | |||
There was a problem hiding this comment.
@christophebedard meta: I wonder if it'd make sense to change expose_action to automatically register "underscore" and "dash" aliases. CC @ivanpauno @wjwwood.
There was a problem hiding this comment.
Even if we did this (just this pr or the automatic mapping) I really think we should pick one way and stick with it.
Ideally there wouldn't be another way to do it.
I'm slightly 👎 to changing the documentation to use underscores after merging this, if we use dashes everywhere else.
There was a problem hiding this comment.
I'm slightly to changing the documentation to use underscores after merging this, if we use dashes everywhere else.
underscores are used everywhere else. This one action is the only one that uses dashes (https://github.com/ros2/ros2_documentation/pull/539/files#r388486312)
There was a problem hiding this comment.
In that case I'd say 👍 for this change and doc change but less so for the automatic mapping. Again preferring one good way to do things.
I'd even be in favor of deprecating the use of dashes.
But I could be convinced otherwise on each of those points I think.
There was a problem hiding this comment.
picking one and sticking with it does make sense, as does deprecating the current name with the dashes (although I'm not sure where in launch_ros I'd put a warning message for it).
There was a problem hiding this comment.
Even if we did this (just this pr or the automatic mapping) I really think we should pick one way and stick with it.
That's fair. IIRC aliases had this purely cosmetic purpose back when we first thought about this problem. Specifically, underscores are somewhat less aesthetically pleasing in XML than dashes. Accepting both does not hurt format translation. But we can be strict too.
We do not enforce one tag per entity though. Maybe we should.
picking one and sticking with it does make sense, as does deprecating the current name with the dashes
There's no support for deprecation, but it's easy enough to add it. If we want to be strict, we can add a list of deprecated aliases to expose_* decorators. We can register the same parsing function for those aliases but decorated to emit a deprecation warning.
There was a problem hiding this comment.
Sounds like deprecating isn't worth it. We can just change the documentation, which is probably enough.
I actually don't mind if there is the ability to use dash or underscore automatically that much, but I do think we should be consistent in the documentation and avoid dashes some place and underscores in others.
Either way sounds like this pr is good and the doc changes are a good idea too.
There was a problem hiding this comment.
Sounds like deprecating isn't worth it. We can just change the documentation, which is probably enough.
Yeah, I'm tempted to just wipe it. Specially if it's just one. But I will say that, eventually, we'll need tick-tock deprecation cycles here too.
There was a problem hiding this comment.
Oh maybe I wasn't clear when I said deprecation wasn't worth it. I was thinking we'd just leave it but document that you should use underscores. So never remove it.
There was a problem hiding this comment.
Ooh. Yeah. I misunderstood you. I'm slightly inclined towards enforcing rather than documenting. None of this documentation is available online just yet. Until then, most people won't read it.
It doesn't have to happen in this PR though.
|
CI
|
|
Alright, this one's ready to go. |
Closes #142
Once this is approved, I will update the documentation to use the alias with the underscores (https://github.com/ros2/ros2_documentation/pull/539/files#r409568794).
Signed-off-by: Christophe Bedard bedard.christophe@gmail.com