search

hbanzhaf / steering_functions

Apache License 2.0
229 stars 94 forks source link

Changed to proper namespace 'steering', removed 'using namespace' in header files and cleaned up includes #15

Closed vebjornjr closed 2 years ago

vebjornjr commented 2 years ago

As discussed in #14 (a long time ago), I added everything in this library into a namespace and changed the namespace name from 'steer' to 'steering' at the same time. I also removed 'using namespace' from header files as this is bad practice in C++ (pollutes the namespace of everybody including the header file). I also cleaned up the includes directives while I was at it.

A version bump is needed due to these breaking changes. Should I do it in this PR or do you that yourself?

vebjornjr commented 2 years ago

Builds and all tests are successful on ROS Noetic.

hbanzhaf commented 2 years ago

@Timple Could you please review this PR?

hbanzhaf commented 2 years ago

@vebjornjr Could you increase the version number to 0.1.1 in the package.xml and add a git-tag with the version number?

Timple commented 2 years ago

Looks good to me!

And our tests succeed when prepending with steering::. So I'm fine.

vebjornjr commented 2 years ago

@hbanzhaf Done. Anything else that needs to be done?

Timple commented 2 years ago

As far as I know, you cannot attach tags to a PR. So after merging @hbanzhaf would still need to tag the commit with the version bump.

hbanzhaf commented 2 years ago

Tag was added. Thanks for your contribution.