PDBQ

Merge pdbq:add-query-lp-milestones into pdbq:main

  1. Git
  2. lp:pdbq
  3. add-query-lp-milestones
  4. Merge into main
Proposed by Bryce Harrington
Status: Merged
Merge reported by: Bryce Harrington
Merged at revision: 82dbf4521a515ab7fc79f98b8e40898a3f1d2ffa
Proposed branch: pdbq:add-query-lp-milestones
Merge into: pdbq:main
Diff against target: 507 lines (+289/-69)
4 files modified
.pylintrc (+3/-0)
pdbq/lp.py (+14/-15)
tests/test_lp.py (+78/-54)
workers/query-lp-milestones (+194/-0)
Reviewer Review Type Date Requested Status
Athos Ribeiro (community) Approve
Canonical Server Reporter Pending
Review via email: mp+440364@code.staging.launchpad.net

Description of the change

Adds to PDBQ a worker to collect the milestones for a given Ubuntu release, and some associated cleanups.

The tests for PDBQ's workers are integrated with the script and can be invoked like this:

    $ ./workers/query-lp-milestones --test

This performs a smoketest against live Launchpad, to confirm that the scripts' EXAMPLE_INPUT generates the EXAMPLE_OUTPUT. Of course, this data is ephemeral since the official milestones can change over time, and eventually jammy will become unsupported. But at least for development smoketesting purposes, this is acceptable, and can be easily updated in the future as needed.

I've also added a readonly option for Launchpad and used it in the smoketest. All the PDBQ workers can operate in this mode so I plan to use this option fairly liberally.

To post a comment you must log in.
Revision history for this message
Athos Ribeiro (athos-ribeiro) wrote :

Thanks Bryce. LGTM. I added a nitpick/question below, but shouldn't be a blocker here.

review: Approve
Revision history for this message
Bryce Harrington (bryce) wrote :

Thanks for the review. I've answered the question, but happy to continue that discussion more if there's a way to make the code still better.

Revision history for this message
Athos Ribeiro (athos-ribeiro) wrote :

>
>No, actually this is so that the argument can be optional when running this with the --test option. I wasn't able to find a cleaner solution, and definitely open to suggestions as I plan to reuse this worker's code style in all the other workers.

Oh, I see.

LGTM then. IF you really want to handle that case, you could add
subparsers for the regular workflow and for the test one, and use the
first as the default subparser (in case no subparsers are selected).

But then we would most likely be overengineering the whole thing. +1 for
merging as is :)

Thanks for clearing things out!

--
Athos Ribeiro

There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: