Adapt to the new 0.7.19 _cli.make_arg_parser signature #3
Conversation
| @@ -12,7 +12,7 @@ repos: | |||
| hooks: | |||
| - id: python-check-blanket-noqa | |||
| - repo: https://github.com/timothycrosley/isort | |||
There was a problem hiding this comment.
we have been using https://github.com/PyCQA/isort, wondering if there is a difference
There was a problem hiding this comment.
No difference, this is just the old URL, which now jumps to PyCQA. I do not think it is worth changing it at this stage. Eventually I would possibly migrate everything over to ruff, but I would not really look into it unless it gives problems
| @@ -50,12 +50,14 @@ jobs: | |||
|
|
|||
There was a problem hiding this comment.
i wonder if there should be a test matrix or something for <0.7.19 and >=0.7.19 so that it's captured
There was a problem hiding this comment.
Good point. This is implicitly covered by the different python versions, because older python versions do not support 0.7.19, while the new ones do pull the latest.
Feel free to open a separate PR if you have suggestions to improve this, but for now I would move on as is, to release a version that solves the problem.
There was a problem hiding this comment.
That is good to know! I'd have maybe added a comment in test.yaml about that or something, for auditing purposes.
Thank you for fixing! I only added my reviews since it is OS and I wanted to share some of the burden, I really appreciate this package.
| enabled_parserplugins = mdformat.plugins.PARSER_EXTENSIONS | ||
| enabled_codeformatters = mdformat.plugins.CODEFORMATTERS | ||
| arg_parser = mdformat._cli.make_arg_parser(enabled_parserplugins, enabled_codeformatters) | ||
| if hasattr(mdformat.plugins, "_PARSER_EXTENSION_DISTS"): |
There was a problem hiding this comment.
sometimes try/except for these situations is more performant that checking hasattr, though probably unnecessary in this case.
There was a problem hiding this comment.
Yes, I do not think it matters much in terms of performance here. Both options would be fine, but I prefer a bit more the if/else branching strategy over the try/except when there are two options which are hierarchically equivalent.
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
Close #2 - Adapt to changes introduced in the
_cli.make_arg_parserfunction in the 0.7.19, with backwards compatibility in mind.