search

colcon / colcon-cmake

Extension for colcon to support CMake packages
http://colcon.readthedocs.io
Apache License 2.0
16 stars 25 forks source link

fix return type (rc is of type int in build.py) #33

Closed dirk-thomas closed 5 years ago

dirk-thomas commented 5 years ago

Follow up of #32.

@paulbovbel FYI

paulbovbel commented 5 years ago

Isn't this basically what was there before https://github.com/colcon/colcon-cmake/commit/7ac46cc59032682babbb95ad2ba30c1f89b755fe?

EDIT: Maybe not, just trying to sort of python truthiness in my mind.

dirk-thomas commented 5 years ago

No, not exactly.

Initially it was checking for if rc which was always True since rc is an instance of subprocess.CompletedProcess. So the loop always stopped after the first target.

With the recent change I fixed the condition to check if rc and rc.returncode but by then returning rc.returncode the caller of the function wasn't happy which still expects that if something is returned it to be a subprocess.CompletedProcess.

paulbovbel commented 5 years ago

Thanks, I'll update our system once you cut a release.

dirk-thomas commented 5 years ago

I'll update our system once you cut a release.

:ship:

MatteoRagni commented 5 years ago

May I suggest to update the title of this pull request with something a little more easy to find? Something like Fix Return type (rc is of type int in build.py)