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

adjust GitHub actions to test Django versions #7910

Closed

Conversation

terencehonles
Copy link
Contributor

Description

This PR adds testing different Django versions which looks like it was dropped in #7903

@terencehonles terencehonles force-pushed the update-github-actions branch 2 times, most recently from 878cf60 to 8b3b0ce Compare April 7, 2021 02:14
@terencehonles terencehonles force-pushed the update-github-actions branch from 8b3b0ce to 267b97d Compare April 7, 2021 02:39
@terencehonles
Copy link
Contributor Author

It looks like the dist and base tests are now failing on main because Django 3.2 was released. I created the separate PR if needed #7911

@terencehonles terencehonles force-pushed the update-github-actions branch from 267b97d to e27b29f Compare April 7, 2021 02:46
Comment on lines -50 to -53
python setup.py bdist_wheel
rm -r djangorestframework.egg-info # see #6139
tox -e base,dist,docs
tox -e dist --installpkg ./dist/djangorestframework-*.whl
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This appears to be no longer needed

Comment on lines -44 to -45
ENV_PREFIX=$(tr -C -d "0-9" <<< "${{ matrix.python-version }}")
TOXENV=$(tox --listenvs | grep "^py$ENV_PREFIX" | tr '\n' ',') tox
Copy link
Contributor Author

@terencehonles terencehonles Apr 7, 2021

Choose a reason for hiding this comment

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

This will be filled in by tox-gh-actions and with ymyzk/tox-gh-actions#60 it could specify everything.

@terencehonles terencehonles force-pushed the update-github-actions branch from e70175b to 475bfc4 Compare April 7, 2021 19:02
TOX_EXIT=0
tox || TOX_EXIT=$?

if [[ $DJANGO == '3.2' && ${{ matrix.python-version }} == '3.9' ]]; then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This could be done as before, but I believe it would not be run if the first fails. This should allow both to run and report an error if either fails.

@adamchainz
Copy link
Contributor

adamchainz commented Apr 16, 2021

I purposefully dropped this in #7903, it was not an accident. The tests run in decent time just splitting by Python version. Splitting by Django version uses a lot more compute power for not much gain. It also requires a lot more config, as you have here, which often ends up bitrotting or getting the grid wrong.

@terencehonles
Copy link
Contributor Author

terencehonles commented Apr 16, 2021

Ok, it wasn't obvious that was the case since there's no comment in the PR that it was intentional. Isn't the gain that different versions of Django are tested for compatibility though? Not sure if you're arguing it's stable enough or that people make a point to stay up to date with Django.

@adamchainz adamchainz closed this Apr 19, 2021
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