launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #29525
Re: [Merge] ~artivis/lpcraft:feature/ros-plugins into lpcraft:main
Jeremie, thanks for the merge proposal.
CI currently fails as there are two uncovered lines, see buildlog below (below `unmerged commits`):
```
:: lpcraft/plugins/plugins.py 284 2 64 1 99% 482, 491
```
Could you please fix these?
As lpcraft is both the CI runner for Launchpad and a standalone tool, I'd like to know how you are planning to use it. Do you use or plan to use it in Launchpad CI?
I am asking as this would influence the plugin story. For a standalone lpcraft we could easily add setuptools based plugins ( https://pluggy.readthedocs.io/en/stable/#loading-setuptools-entry-points ), so we could have the plugin in a standalone plugin package. For Launchpad CI this is more elaborate, and currently out of scope.
I am discussing these options, as at least for me ROS is a business domain I have never touched before, and including these plugins means I (we) have to maintain them.
Fortunately, the code looks pretty straightforward. I will discuss this MP at today's standup and will get back to you.
Afterwards, I will also give you a detailed review. At very least you would need to add some descriptive docstrings which explain the used business terminology.
Also, does this MP depend on https://code.launchpad.net/~artivis/lpcraft/+git/lpcraft/+merge/435370 or would merging this MP on its own already be helpful?
--
https://code.launchpad.net/~artivis/lpcraft/+git/lpcraft/+merge/435371
Your team Launchpad code reviewers is requested to review the proposed merge of ~artivis/lpcraft:feature/ros-plugins into lpcraft:main.
References