search

ros-visualization / python_qt_binding

http://wiki.ros.org/python_qt_binding
BSD 3-Clause "New" or "Revised" License
34 stars 54 forks source link

add sip5 binding install directory check #95

Closed acxz closed 3 years ago

acxz commented 3 years ago

resolves #80 affected downstream issues: https://github.com/ros-noetic-arch/ros-noetic-python-qt-binding/issues/2 https://github.com/ros-melodic-arch/ros-melodic-rviz/issues/9

Basically adds the sip5 default binding install directory to the check, since this will be the new place of bindings and some distros have already switched causing errors on those systems without adding this check. Added comments in code for clarity.

acxz commented 3 years ago

@dirk-thomas can you take a quick look at this patch?

acxz commented 3 years ago

@dirk-thomas @claireyywang @sloretz it has been more than 2 week since this is open. Can you provide closure to this PR? It'll be quick ;)

dirk-thomas commented 3 years ago

I am not maintaining this repository anymore. That is why I assigned the new maintainers to review this PR.

acxz commented 3 years ago

@claireyywang @sloretz it has been around a month since this is open. Can you provide closure to this PR?

acxz commented 3 years ago

@claireyywang @sloretz friendly ping

acxz commented 3 years ago

@claireyywang @sloretz friendly ping

claireyywang commented 3 years ago

@acxz Sorry about the delay, I'll take a look soon!

acxz commented 3 years ago

@claireyywang @sloretz it has been around 2 months since this PR has been opened, sorry to keep bugging you guys about it, but it really is a quick fix. It would be much appreciated if you guys can provide closure to this PR. Thank you.

acxz commented 3 years ago

@claireyywang @sloretz would be much appreciated if you can take a glance at this.

acxz commented 3 years ago

@claireyywang @sloretz another ping.

acxz commented 3 years ago

@claireyywang @sloretz it has been a couple months now if you guys could take just a quick look at this, it would be much much appreciated!

acxz commented 3 years ago

@claireyywang @sloretz another ping.

:(

acxz commented 3 years ago

thank you for considering my PR!

mikepurvis commented 3 years ago

@sloretz Would it be possible to get a ROS 1 release which includes this change? We can use it from the hash in the short term, but we need it for the environment we're building in.

sloretz commented 3 years ago

@mikepurvis released in https://github.com/ros/rosdistro/pull/30232 and https://github.com/ros/rosdistro/pull/30233

mikepurvis commented 3 years ago

Awesome, thanks!