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

Use fissix in order to support newer python versions #17

Merged
merged 12 commits into from
Apr 16, 2024

Conversation

dosas
Copy link
Contributor

@dosas dosas commented Jan 18, 2021

#16

@jayvdb
Copy link
Contributor

jayvdb commented Jan 19, 2021

Looks like there are two expected strings which need to be adjusted to match what fissix produces.

@dosas
Copy link
Contributor Author

dosas commented Jan 20, 2021

Looks like this is the same problem as described here #15

@cclauss
Copy link
Contributor

cclauss commented Jun 27, 2022

@dosas Would you be willing to add the changes from #21 and then I can close #21? I think it would be easier for maintainers to review and accept the changes in this PR if it included working continuous integration testing.

setup.cfg Outdated Show resolved Hide resolved
@dosas
Copy link
Contributor Author

dosas commented Jun 27, 2022

@cclauss Done.

@cclauss
Copy link
Contributor

cclauss commented Jun 27, 2022

Cool. Thanks. Let’s see what happens when a maintainer gives the jobs permission to run.

@dosas
Copy link
Contributor Author

dosas commented Jun 27, 2022

It will probably fail due to #15 ;)

@cclauss
Copy link
Contributor

cclauss commented Mar 29, 2023

Please rebase.

@jayvdb
Copy link
Contributor

jayvdb commented Mar 29, 2023

ping @dosas

@dosas
Copy link
Contributor Author

dosas commented Mar 31, 2023

pong

@dosas dosas requested a review from cclauss March 31, 2023 20:28
@schollii
Copy link
Collaborator

schollii commented Apr 11, 2023

Looks like this is still WIP? Also there are conflicts to resolve. Let me know when this is ready for review, I look forward to merging this.

@cclauss
Copy link
Contributor

cclauss commented Apr 8, 2024

@schollii Your review, please.

@schollii
Copy link
Collaborator

schollii commented Apr 10, 2024

I'm not seeing the tox test suite results proving that everything works fine after the changes, are they somewhere else? Sorry it's been a while since I looked at pytest cicd here.

Update : I can see "Checks waiting for approval" and a button "Approve and run", that's probably what will run tox but can someone confirm that's what I have to do, I don't want to approve PR until I see this.

@nicoddemus
Copy link
Member

It needed approval to run the test suite (GH does that for first contributors to avoid crypto-spam).

Just gave approval, CI is running.

Copy link
Collaborator

@schollii schollii left a comment

Choose a reason for hiding this comment

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

Changes look good and all tests pass, so good to go!

Thank you @ich and @dosas for doing this.

@cclauss
Copy link
Contributor

cclauss commented Apr 10, 2024

Next steps?

@nicoddemus
Copy link
Member

I guess all that is left is for @schollii to merge it?

@schollii schollii merged commit 2d560b6 into pytest-dev:master Apr 16, 2024
18 checks passed
@dosas dosas mentioned this pull request Apr 16, 2024
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.

5 participants