-
Notifications
You must be signed in to change notification settings - Fork 49
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
[noetic] deprecate h for hpp #34
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a diff of the .h and .hpp before and after this commit, respectively. The only differences between them are the name of the includes. However, I also think the include header guards should be changed as well to match the new .hpp extension. Otherwise this looks good to me.
include/filters/filter_base.hpp
Outdated
#ifndef FILTERS_FILTER_BASE_H_ | ||
#define FILTERS_FILTER_BASE_H_ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The include guard should be:
#ifndef FILTERS_FILTER_BASE_H_ | |
#define FILTERS_FILTER_BASE_H_ | |
#ifndef FILTERS_FILTER_BASE_HPP_ | |
#define FILTERS_FILTER_BASE_HPP_ |
The same goes for all of the other header files below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated include guards in 552f6b3
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
c06c92c
to
552f6b3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to merge with one small typo unrelated, but we might as well fix it.
Co-Authored-By: Tully Foote <tfoote@osrfoundation.org>
Could the new headers be back-ported to melodic ? currently it is inconvenient to get a melodic/noetic compatible code that compiles without warnings |
I use #if ROS_VERSION_MINIMUM(1, 15, 0)
#include <filters/filter_chain.hpp>
#else
#include <filters/filter_chain.h>
#endif to get melodic/noetic compatible code. |
This is another PR created with the goal of shrinking the diff in #32. It deprecates
.h
files in favor of.hpp
in the same way it was done forclass_loader
. Igit mv
'd the .h files to .hpp in one commit, added new.h
files with the deprecation warning in another, and fixed all the deprecation warnings in this package in the last commit.Deprecation warnings prior to fixing; proof the header warnings work
grep to make sure all includes use .hpp and not .h