Skip to content
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

Catch2: support v3 #115

Merged
merged 7 commits into from
Sep 12, 2023
Merged

Catch2: support v3 #115

merged 7 commits into from
Sep 12, 2023

Conversation

jnewb1
Copy link
Collaborator

@jnewb1 jnewb1 commented Sep 11, 2023

original work from: #107

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great @jnewb1, thanks! Can you please also add a CHANGELOG entry?

cls,
executable: str,
harness_collect: Sequence[str] = (),
) -> bool:
) -> Optional[str]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to type this correctly, we can either use an enum, or Literal:

Suggested change
) -> Optional[str]:
) -> Optional[Literal["v2", "v3"]]:

@@ -123,11 +148,16 @@ def run_test(
return [failure], output

def _parse_xml(
self, xml_filename: str
self, xml_filename: str, catch_version: str
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self, xml_filename: str, catch_version: str
self, xml_filename: str, catch_version: Literal["v2", "v3"]

@nicoddemus
Copy link
Member

@jnewb1 this is your 2nd high-quality PR to this repository, would you like to be added as a maintainer? It would not entail much more responsibility, but it would give you permission to also review other PRs and handle issues (this is a low volume repository).

@jnewb1
Copy link
Collaborator Author

jnewb1 commented Sep 12, 2023

@jnewb1 this is your 2nd high-quality PR to this repository, would you like to be added as a maintainer? It would not entail much more responsibility, but it would give you permission to also review other PRs and handle issues (this is a low volume repository).

sure, you can add me as a maintainer!

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@nicoddemus
Copy link
Member

sure, you can add me as a maintainer!

Awesome! Sent you an invite. 👍

@nicoddemus nicoddemus merged commit bd997b4 into pytest-dev:master Sep 12, 2023
7 checks passed
@jnewb1 jnewb1 mentioned this pull request Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants