-
-
Notifications
You must be signed in to change notification settings - Fork 29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
yaml literals #70
yaml literals #70
Conversation
Thanks! The quality failure is because of missing blank lines: --- src/scriv/literals.py 2022-11-28 11:30:03.217489 +0000
+++ src/scriv/literals.py 2022-11-28 11:31:41.120974 +0000
@@ -13,10 +13,11 @@
try:
import yaml
except ImportError: # pragma: no cover
yaml = None # type: ignore
+
def find_literal(file_name: str, literal_name: str) -> Optional[str]:
"""
Look inside a file for a literal value, and return it.
@@ -111,10 +112,11 @@
if isinstance(current_object, str):
return current_object
return None
+
def find_yaml_value(data: MutableMapping[str, Any], name: str) -> Optional[str]:
"""
Use a period-separated name to traverse a dictionary.
Only string values are supported. I've improved the check on main so that it would be more evident in the future. Can you write some tests? The toml support has ones you can use as a model. Also, a scriv entry? :) |
safe_load will take an open file Co-authored-by: Ned Batchelder <ned@nedbatchelder.com>
One is gone with the superfluous function, one blank line inserted.
Thanks for making those easier to understand. I gave it a try here with little experiance and a simple text editor, so the likelihood of triggering the quality bots is high.
I feared You'd say that 😄 I will try right now.
How could I forget? |
I must be missing something simple: |
You need to add types-yaml to requirements/quality.in, then run |
|
It looks like you ran |
I will try |
I will pick that up tomorrow (it was a long day) . |
Sorry about the tests on your fork. I've fixed that test on master. Get that code, and your tests should pass. Ask me anything you need, I really appreciate you putting in the work. |
Good morning Ned,
I have added 5a9c278 The matrix test still don't run. |
I can now fail on purpose: and also run successful tests: I tried (modeled after toml) testing for installation of I will now have a look at |
I beleive Your code assumes a github repo should end in While my url line is:
From the test You even say that, but then gitinfo.py does not reflect that. did I miss a patch ? |
Your pull request is helping me find all sort of problems to fix! Merge master again to fix the test_gitinfo problem, and removing your debug prints will solve one of the others. I haven't looked at the yaml failure. |
on my way.. |
I am off the keyboard for 15 mins. ... celebratory coffee |
This is the one , I still need help with: |
Looks like it's just a mismatch in the case of "yaml" between the message and the regex matching the message:
|
Thanks! |
Ned, (scriv_venv) root@buster:~/scriv/scriv# grep -ni yaml requirements/*.in
requirements/quality.in:19:types-pyyaml
requirements/test.in:11:pyyaml
(scriv_venv) root@buster:~/scriv/scriv# Should I remove that or put in quality.in ? |
I think you were right to put pyyaml into test.in I've fixed the badge creation on master, if you merge one more time, your Coverage check will pass The docs look fine. I think I will take an editing pass over that whole section, since the first sentence mentions "variables", which is a holdover from the only-.py days. I think this might be ready to go! |
I don't mind though.
GREAT! Mind You - I have written an eMail to You with three little beginner Questions I have. If You like some other way of doing that chat vid-conference, whatever... I am happy too. Cheers, Florian! |
Coverage badge run succeeds! |
Thanks!! |
#69