codelists add command
#294
Conversation
Jongmassey
force-pushed
the
Jongmassey/codelists-add
branch
2 times, most recently
from
5250f18 to
d67b09f
Compare
Jongmassey
force-pushed
the
Jongmassey/codelists-add
branch
2 times, most recently
from
b9e2d56 to
e421132
Compare
Jongmassey
force-pushed
the
Jongmassey/codelists-add
branch
from
9bba87e to
3f29fdb
Compare
Jongmassey
force-pushed
the
Jongmassey/codelists-add
branch
from
3f29fdb to
013f1e3
Compare
Allow users to add OpenCodelists codelists to the codelists.txt file by URL, allowing for common variations from the "base" URL
Jongmassey
force-pushed
the
Jongmassey/codelists-add
branch
from
013f1e3 to
d73fe6f
Compare
A lot of the logic of `update` is needed by `add`: * codelist file parsing (indirectly) * file downloading * manifest handling This commit pulls out those needed parts into their own functions, and pulls out parse_codelist_file's line-parsing logic into its own function. This allows us to check the line we're about to add to the codelists file is valid before doing so, check that once we add the line to the codelists file that it is valid, leave pre-existing codelists and their corresponding manifest file entries intact.
Jongmassey
force-pushed
the
Jongmassey/codelists-add
branch
from
d73fe6f to
ce85bfa
Compare
There was a problem hiding this comment.
Kent Beck is famous for saying "First make the change easy, then make the easy change". I think you may have done it the other way around, but it's definitely looking good. Thank you for responding to the various pieces of feedback.
I'm approving this because the changes I've suggested are very minor, but please be aware that I haven't run the cli tool locally to test the change. If you'd like me to do that, please can you tell me how to do that, because I can't find the docs.
Also, I believe we use semantic commits on this repo (i.e feat:), so you'll want to rename the commits before merging to release a new version.
| if OPENCODELISTS_BASE_URL not in codelist_url: | ||
| exit_with_error(f"Unable to parse URL, {OPENCODELISTS_BASE_URL} not found") | ||
|
|
||
| if not codelists_dir: |
There was a problem hiding this comment.
Up to you whether you change this, but I notice that there's a get_codelists_dir function later on in the file that you could make use of here.
| codelists_path /= "codelists" | ||
| codelists_file = codelists_path / "codelists.txt" | ||
| prior_codelists = codelists_file.read_text() | ||
| for codelist in prior_codelists.split("\n"): |
There was a problem hiding this comment.
This for-loop and call the requests_mock is no longer needed.
| codelists_path /= "codelists" | ||
| codelists_file = codelists_path / "codelists.txt" | ||
| prior_codelists = codelists_file.read_text() | ||
| for codelist in prior_codelists.split("\n"): |
There was a problem hiding this comment.
This for-loop and call the requests_mock is no longer needed.
Allow users to add OpenCodelists codelists to the codelists.txt file by URL.
Fixes #295