Merge ppa-dev-tools:add_unpack_to_dict into ppa-dev-tools:main
Status: | Merged |
---|---|
Merge reported by: | Bryce Harrington |
Merged at revision: | 8a1b18cebbfd84db7011df6569febb757531fa9e |
Proposed branch: | ppa-dev-tools:add_unpack_to_dict |
Merge into: | ppa-dev-tools:main |
Diff against target: |
312 lines (+300/-0) 2 files modified
ppa/dict.py (+183/-0) tests/test_dict.py (+117/-0) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Lena Voytek (community) | Approve | ||
Canonical Server | Pending | ||
Canonical Server Reporter | Pending | ||
Review via email: mp+437970@code.staging.launchpad.net |
Description of the change
ppa-dev-tools needs to be able to parse strings of comma-separated values. There are three different types of these strings: Argument values (i.e. --architectures
This branch adds an "unpack_to_dict()" helper routine that strives to solve all these needs. It supports overriding the markers to allow customizing the parsing behavior to cover the aforementioned use cases. Hopefully it'll be useful in other situations too.
I recognize one issue with trying to solve multiple special cases with one generic routine is there can be exceptions for one case that interfere with others, and handling them can make the generic routine less useful than just having separate routines for each case. Thankfully, I haven't yet run into any exceptions this can't handle, but admittedly I haven't used this in practice very widely. To mitigate this problem, I've implemented an extensive set of test cases that can be expanded as exceptions and new use cases are discovered to ensure that the existing use cases remain correctly handled.
The tests can be invoked via:
$ pytest-3 ./tests/
There is also a smoketest that demonstrates parsing of some different real-world examples, to give a better feel for how this will be used in practice.
$ python3 ./ppa/dict.py
There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.
The additions here look good to me. I also ran the unit tests on my system and everything passed. I added some comments for nitpicks + code clarity below