ppa-dev-tools

Merge ppa-dev-tools:wait-watch into ppa-dev-tools:main

  1. Git
  2. lp:ppa-dev-tools
  3. wait-watch
  4. Merge into main
Proposed by Bryce Harrington
Status: Merged
Merge reported by: Bryce Harrington
Merged at revision: fca5c1ea787f886d1a78d26ba7d1b97766b2cde9
Proposed branch: ppa-dev-tools:wait-watch
Merge into: ppa-dev-tools:main
Diff against target: 146 lines (+61/-5)
3 files modified
ppa/ppa.py (+8/-2)
scripts/ppa (+27/-3)
tests/test_scripts_ppa.py (+26/-0)
Reviewer Review Type Date Requested Status
Christian Ehrhardt  (community) Approve
PpaDevTools Developers Pending
Canonical Server packageset reviewers Pending
Canonical Server Reporter Pending
Review via email: mp+451001@code.staging.launchpad.net

Description of the change

Adds a --watch argument that gives the appearance of waiting "in place". So, instead of an ever scrolling list of status updates, the user sees just the current state of everything. This is done by clearing the screen before each update.

I want to get this feature introduced so people can try it out, before adding further refinements, because while there's a few ideas we can already think of, the best way to know is to see how it works in practice. Moreover, test coverage for command_wait() is practically non-existent, and I'd like to take some time to get proper testing in place before the implementation gets too sophisticated.

Also fills in some missing docs and light testing for just the command line args themselves.

To post a comment you must log in.
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Maybe I miss the point, sorry in that case!
But either way I hope my input is useful to you.

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

I've incorporated the suggested changes from the review, can you take another look?

Thanks again!

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Much better +1 - thank you

On this aspect:

> Honestly, I'm a bit tempted to just make this the standard default behavior since it works
> so well, and make the prior behavior available via a --log option, or maybe only when the
> script is run without a tty (i.e. `ppa wait ... | tail -f`). WDYT?

I like this (default with tty = --watch, default without = --log) - the options allow to force the other.
+1 for doing that, if you change that logic but nothing else you do not need to go another review round.

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

Thanks, I've reversed the behavior, switched the option to --log, verified the tests still pass and landed the branch to main.

$ git merge --ff-only wait-watch
Updating 20533a7..3f278c0
Fast-forward
 ppa/ppa.py | 10 ++++++++--
 scripts/ppa | 30 +++++++++++++++++++++++++++---
 tests/test_scripts_ppa.py | 18 ++++++++++++++++++
 3 files changed, 53 insertions(+), 5 deletions(-)
$ git push origin
Enumerating objects: 21, done.
Counting objects: 100% (21/21), done.
Delta compression using up to 12 threads
Compressing objects: 100% (12/12), done.
Writing objects: 100% (14/14), 2.19 KiB | 2.19 MiB/s, done.
Total 14 (delta 10), reused 0 (delta 0), pack-reused 0
To git+ssh://git.launchpad.net/ppa-dev-tools
   20533a7..3f278c0 main -> main

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: