Code review comment for ~mertkirpici/juju-lint:lp/1987951

Revision history for this message
Mert Kirpici (mertkirpici) wrote :

Hey again, so first of all I did go ahead and file LP#1989673 for the first item you mentioned.
https://bugs.launchpad.net/juju-lint/+bug/1989673

After some thought I decided to deprecate the `--dump-state` rather than directly removing it, so this newly pushed update is mainly dealing with that. It:

- introduces a new argparse.Action subclass for deprecation
- marks `--dump-state` as nargs="*" therefore no matter with how many arguments it is called, it will be valid(and no-op, just log a warning message)
- fixes breaking unittests and adds a test to ensure %100 coverage
- re-renames some tests and function names for consistency's sake

Here is the `make test` output:
https://pastebin.ubuntu.com/p/rqd5tcDVS2/

I do have one request though. If you think this is good to go, could you let me know before merging so that I can squash the commits and add explanation for the changes in the commit message to clear up the intention behind this change?

« Back to merge proposal