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

Fix #1092 Update Python/Django versions in tests #1093

Merged

Conversation

pauloxnet
Copy link
Contributor

No description provided.

setup.cfg Outdated Show resolved Hide resolved
@browniebroke
Copy link
Contributor

Is there anything left to do? How can we help to get this merged?

@jschneier
Copy link
Owner

There are a couple things I think we should update before merging.

  1. install_requires does not properly drop Django versions
  2. CHANGELOG updates should not include 1.12.4, I wouldn't release a Django or Python dropping version as a patch (did that once and was called out for it, rightly)

@pauloxnet pauloxnet force-pushed the feature/python-django-latest-versions branch from 2356203 to 8282339 Compare July 18, 2022 06:27
@pauloxnet
Copy link
Contributor Author

There are a couple things I think we should update before merging.

Thanks for the feedback, I updated this PR.

  1. install_requires does not properly drop Django versions

I updated the Django versions in install_requires

  1. CHANGELOG updates should not include 1.12.4, I wouldn't release a Django or Python dropping version as a patch (did that once and was called out for it, rightly)

I updated to version 1.13

I also rebased the branch to let you merge it.

@jschneier
Copy link
Owner

Sorry, one more thing. Can you please revert the change to __version__ and setting the version in CHANELOG.rst. For the second you can just put XXXX-XX-XX.

@pauloxnet pauloxnet force-pushed the feature/python-django-latest-versions branch from cec4893 to ca70489 Compare August 4, 2022 15:47
@pauloxnet
Copy link
Contributor Author

Can you please revert the change to __version__ and setting the version in CHANELOG.rst. For the second you can just put XXXX-XX-XX.

I reverted the changes on the version, I rebased to the branch master to resolve a merge conflict and I also added the support to Django 4.1 which was published yesterday.

@jschneier
Copy link
Owner

@pauloxnet thanks for the persistence. There is now a test failure.

@pauloxnet
Copy link
Contributor Author

pauloxnet commented Aug 5, 2022

@pauloxnet thanks for the persistence. There is now a test failure.

@jschneier Actually I had forgotten how old this PR is. I realized that in the meantime Django 2.2 is no longer supported, so I updated this PR. Locally the tests pass, I hope they pass even remotely. Unfortunately, as first-time Contributor for this project they will not automatically start, so I will have to wait for your approval to be sure.
Anyway thanks for your patience.

@jschneier jschneier merged commit acb735e into jschneier:master Aug 5, 2022
@jschneier
Copy link
Owner

Merged, thanks for the patience.

@pauloxnet
Copy link
Contributor Author

Thanks to you
I am happy to have been able to contribute

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.

3 participants