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

Improve tests #260

Merged
merged 4 commits into from Oct 3, 2022
Merged

Improve tests #260

merged 4 commits into from Oct 3, 2022

Conversation

quodlibetor
Copy link
Contributor

This PR is just a small thanks for providing the library, I don't need you to merge it on my behalf. Let me know if you'd like me to change anything.

This has two commits:

  • Add SECRET_KEY to django config

    When I did poetry run python runtests.py Django complained that the
    SECRET_KEY was required. After I ran tests once it doesn't seem to matter, but
    this was a small speedbump that I thought it might be nice for future users to
    not encounter.

  • Configure tox to test against Django 4.1

    Since it's out now and it works.

When I did `poetry run python runtests.py` Django complained that the
SECRET_KEY was required. After I ran tests once it doesn't seem to matter, but
this was a small speedbump that I thought it might be nice for future users to
not encounter.
Copy link
Contributor

@pcraciunoiu pcraciunoiu left a comment

Choose a reason for hiding this comment

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

LGTM just a small note

exclude:
- python-version: 3.7
django-version: 40
- python-version: 3.7
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should move to python 3.10 as the default, and add it to the matrix too. I'm not sure if 3.7 support is still warranted, but I'm fine keeping it for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure how django-ses selects the default Python, but I'm happy to change it. I've added it to the gitlab test suite.

I'm not sure if 3.7 support is still warranted, but I'm fine keeping it for now.

Yeah I wanted to be conservative and I didn't need any changes that would require bumping the version. I'm happy to remove 3.7 from tests and pypi classifiers if you'd like.

Copy link
Contributor

@pcraciunoiu pcraciunoiu left a comment

Choose a reason for hiding this comment

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

Thank you for these improvements!

@pcraciunoiu pcraciunoiu merged commit ac4bec6 into django-ses:master Oct 3, 2022
@quodlibetor quodlibetor deleted the improve-tests branch October 3, 2022 18:57
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.

None yet

2 participants