-
-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat: Add a poetry-specific parser, and a mechanism for selecting it. #33
feat: Add a poetry-specific parser, and a mechanism for selecting it. #33
Conversation
@@ -189,6 +189,7 @@ plugins = coverage_pyver_pragma | |||
|
|||
[coverage:report] | |||
fail_under = 100 | |||
show_missing = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was having trouble figuring out which lines were missing coverage, given the requirement that it be 100% coverage.
pep621_style_authors: List[dict[str, str]] = [] | ||
|
||
for author in config["author"]: | ||
match = re.match(r"(?P<name>.*)<(?P<email>.*)>", author) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I originally implemented this as a straight-copy of the base class's parse_author
. The implementation used a simpler string splitting mechanism to just pull out the name, which is the required field for "author", iirc.
But given the 100% coverage requirement, I wanted to get reuse out of the preexisting test that handles lack of any authors at all, so it felt perhaps better to redirect back to the base class's implementation ultimately.
But I could just as easily add a test and have the simpler impl here.
Is there anything happening with this? |
For what it's worth, i've personally just been depending upon my fork's above revision in the interim |
Sadly that won't work for me since poetry can't use git dependencies in dist versions. Any idea why the workflow reports didn't get reported? @domdfcoding what can we do to get this merged? |
Sorry for the wait. This went under the radar. I've rebased and added some more documentation and one extra test. Once the tests are green I'll get this merged and released. |
thanks! |
Fixes #29