[update] Allow to process single ROS distro, fix 723 #738
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Mikael Arguedas <[email protected]>
dirk-thomas
reviewed
Jan 6, 2020
Codecov Report
@@ Coverage Diff @@
## master #738 +/- ##
==========================================
- Coverage 75.99% 75.82% -0.18%
==========================================
Files 40 40
Lines 3129 3139 +10
==========================================
+ Hits 2378 2380 +2
- Misses 751 759 +8
Continue to review full report at Codecov.
|
mikaelarguedas
force-pushed
the
single_ros_distro
branch
from
fc3da8c to
e61e13f
Compare
|
CI failures seem unrelated to this change as they happen on master as well Edit: CI failures addressed in #739 |
|
I am not the maintainer of this package therefore I unassigned myself as a reviewer. |
|
As you reviewed and asked for changes I thought you were the right person to request a follow up review from. Could you please clarify who the current maintainer is? |
|
|
I’m unlikely to have time to review this soon, but I’ll get to it when I can. @dirk-thomas I think it is reasonable to ask you to finish a review if you start one and the maintainer doesn’t have to be the only person to review the pull request. |
wjwwood
reviewed
Jan 24, 2020
Signed-off-by: Mikael Arguedas <[email protected]>
Sounds good. What do you think of 28bd6d2 ? |
wjwwood
approved these changes
Jan 24, 2020
wjwwood
approved these changes
Jan 24, 2020
| if distribution.get('distribution_status') == 'end-of-life': | ||
| print('Skip end-of-life distro "%s"' % dist_name) | ||
| if dist_name != ros_distro: | ||
| if ros_distro is not None: |
There was a problem hiding this comment.
This logic (the two lines here in combination with the for loop) is a bit tricky, but I think it's correct.
|
The issues in CI are unrelated in my opinion. |
|
Thanks for the express review!
Seems like it, #739 addresses unrelated the CI failures |
gstavrinos
pushed a commit
to gstavrinos/rosdep
that referenced
this pull request
Nov 12, 2020
* Add Suite3 option with Ubuntu Focal (ros-infrastructure#734) Signed-off-by: Shane Loretz <[email protected]> * [update] Allow to process single ROS distro, fix 723 (ros-infrastructure#738) * [update] skip other distro if --rosdistro passed Signed-off-by: Mikael Arguedas <[email protected]> * ignore argument if specified distro doesnt exist * address review comments * update help message for rosdistro Signed-off-by: Mikael Arguedas <[email protected]> * Fix CI and reduce CI time (ros-infrastructure#739) * test newer python * use yaml.safe_load * pin PyYAML version for Python 3.4 * pass user flag to pip when needed * move slow jobs to the top to reduce CI time Signed-off-by: Mikael Arguedas <[email protected]> * Strip Alpine's patch version from OS codename (ros-infrastructure#716) * Resolve Alpine os_version_type using OsDetect * Reduced Alpine OS VERSION to Major.Minor * improve support for null entries (ros-infrastructure#726) * support null for entire OS and not only OS version syntax like 'ubuntu: null' will now be raising ResolutionError with relevant error message instead of InvalidData with obscure error message will also cover syntax like: " ubuntu: '*': null bionic: [foobar] " Signed-off-by: Mikael Arguedas <[email protected]> Co-authored-by: Scott K Logan <[email protected]> * Use DNF installer on RHEL 8 and newer (ros-infrastructure#713) RHEL/CentOS 8 uses DNF by default. * Updates to YUM and DNF (ros-infrastructure#640) Output YUM, DNF and RPM versions with `--all-versions` and fix the format guide's default PM for Fedora. * tests: don't assume euid != 0 (ros-infrastructure#703) If tests rely on the environment being a certain way, they should ensure it meets its expectations. Update the tests to ensure they're testing multiple euid conditions, and setting it explicitly where required. Fix ros-infrastructure#702 Signed-off-by: Kyle Fazzari <[email protected]> * openSUSE package query and install enhancements (ros-infrastructure#729) * Enable PIP installer for openSUSE * openSUSE package detection with RPM capabilities Packages sometimes get renamed and their old name is kept as an rpm capability (like an alias), so the additional flag `--whatprovides` is passed to the `rpm` query. * Fix conditional dependencies when one package uses manifest.xml (ros-infrastructure#737) * Fix conditional dependencies when one package uses manifest.xml Signed-off-by: Shane Loretz <[email protected]> * Return rosdeps as a list to avoid breaking the interface. Co-authored-by: Steven! Ragnarök <[email protected]> * Guard next(inter) (ros-infrastructure#701) * guard next(inter) Fix ros-infrastructure#691 Signed-off-by: artivis <[email protected]> * Handle StopIteration with slightly less line noise. Co-authored-by: Steven! Ragnarök <[email protected]> * [Windows] Add console script entry point (ros-infrastructure#656) * Add console script entry point Add console script entry point for platforms (e.g. Windows) not supporting shebang. * remove scripts entry since we are using console_scripts. * fix bad merge. * fix SKIP_PYTHON_SCRIPTS case. Co-authored-by: Lou Amadio <[email protected]> * Depend on modules packages only to allow co-installability. (ros-infrastructure#750) When the rosdep modules package was split in [ros-infrastructure#731] the module dependencies weren't updated to only depend on modules packages themselves which prevents rosdep modules for python2 and python3 from actually being co-installable. I haven't yet audited to make sure there's no cli usage of these tools in the modules but it's Friday afternoon and I wanted to get this inked before I walked away. When I return to it I'll check for that before proceeding for reviews. * 0.19.0 * also install buildtool_export_depends (ros-infrastructure#753) not adding 'exec_depends' or 'build_export_depends' are both are included inside 'run_depends' Signed-off-by: Mikael Arguedas <[email protected]> * Drop shebang from a non-executable file (ros-infrastructure#755) * add alias for Pop! OS (ros-infrastructure#757) * add alias for Pop! OS Signed-off-by: Mikael Arguedas <[email protected]> Co-authored-by: Shane Loretz <[email protected]> Co-authored-by: Mikael Arguedas <[email protected]> Co-authored-by: Mark Hedley Jones <[email protected]> Co-authored-by: Scott K Logan <[email protected]> Co-authored-by: Kyle Fazzari <[email protected]> Co-authored-by: Bjar Ne <[email protected]> Co-authored-by: Jeremie Deray <[email protected]> Co-authored-by: Sean Yen <[email protected]> Co-authored-by: Lou Amadio <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #723
Handling of
--rosdistroargument in 9915909. Added a print for consistency with skipping EOL but it may be better to remove it.ff38f1e preserves original behavior if the provided rosdistro name is unknown.
With existing rosdistro
With no-existing rosdistro
@wjwwood FYI