enforce version conventions #15774
enforce version conventions #15774
Conversation
|
Do all package manifests at the moment parse without this warning? I prefer option 2 since it doesn't duplicate the logic. |
test/test_build_caches.py
Outdated
| if 'version conventions' in warning: | ||
| errors.append('%s: %s' % (dist_name, warning)) | ||
| else: | ||
| print('WARNING: ' + warning) |
There was a problem hiding this comment.
In both cases (error and warning) it would be valuable to mention the pkg_name. The dist_name is already being printed before so it should be clear to which distro the message belong.
There was a problem hiding this comment.
hmm good point, I didnt print the package name because, so far, every warning/error from the parsing function already contain the package name in the message (e.g. https://github.com/ros-infrastructure/catkin_pkg/blob/master/src/catkin_pkg/package.py#L212) but maybe it's an assumption that won't hold in the future and we should print the name of the package before.
You're right the distribution name doesn't add value here.
Note: Catching the warnings in parse_package_string forces us to duplicate the logic for printing them.
Yes, since ompl has updated their version scheme all packages parse without this warning. The only warnigs we get are for naming conventions for a few python packages that have the same name as the upstream one |
This is a suggestion to address #11852
Option 1 Performs the check of the version format in this scripts directly
Option 2 Parses the warnings from catkin_pkg.parse_package_string and add an error if the warning about version conventions is present
Looking for feedback regarding which approach is preferred