← Back to team overview

launchpad-reviewers team mailing list archive

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