Skip to content

Conversation

@exkson
Copy link
Contributor

@exkson exkson commented Dec 16, 2024

No description provided.

@yezyilomo
Copy link
Owner

Thank you so much for your PR. Adding support for generic relations is a great idea, and I really appreciate the effort you've put into this. Your code looks fantastic overall, and I especially like that you took the time to include tests.

That said, I noticed that the checks are failing for Python versions below 3.9. Since we support Python 3.6 and above, it seems the issue is related to the use of type hints in some parts of your code. To ensure compatibility for users on older Python versions, could you update the code to avoid type hints that are unsupported below Python 3.9?

Once that’s addressed, I’d be happy to merge this. Let me know if you have any questions or need help with the changes. Thanks again for your contribution! .

@exkson
Copy link
Contributor Author

exkson commented Dec 19, 2024

Hello @yezyilomo . Thanks for your message. I understand the problem and i'm trying to fix it asap. Hope it is good now. Do you know any way to test my code against python 3.6 without install another interpreter ?

@yezyilomo
Copy link
Owner

Nice work! Just one small thing: it looks like Model and GenericRelation are still in the imports, even though they’re no longer used. Could you remove those? Thanks!

@yezyilomo
Copy link
Owner

Do you know any way to test my code against python 3.6 without install another interpreter ?

No need to run tests locally for all the Python versions we support, the CI handles that for you! In your local environment, you can just run tests for the Python version you have installed. Once the tests pass locally, push your changes, and the CI will run tests for all supported versions. If anything goes wrong, the checks will fail, and we can review the logs to see what needs fixing. Hope that clears things up.

@yezyilomo
Copy link
Owner

Everything looks good now and all checks have passed, thank you for your contribution @exkson

@yezyilomo yezyilomo merged commit 615d980 into yezyilomo:master Dec 19, 2024
6 checks passed
@yezyilomo yezyilomo mentioned this pull request Dec 28, 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.

2 participants