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

Add requirements for Open edX Redwood #27

Merged
merged 1 commit into from
Aug 1, 2024
Merged

Conversation

mrtmm
Copy link

@mrtmm mrtmm commented Jul 24, 2024

No description provided.

Copy link
Contributor

@fghaas fghaas left a comment

Choose a reason for hiding this comment

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

If I understand correctly, Tutor 18 now runs Open edX on Python 3.11. I think that means we should update the test matrix to include that version, correct?

https://github.com/overhangio/tutor/blob/v18.1.1/tutor/templates/build/openedx/Dockerfile#L27

@mrtmm
Copy link
Author

mrtmm commented Jul 31, 2024

Yep that makes sense, let's see if I added it correctly.

@fghaas
Copy link
Contributor

fghaas commented Aug 1, 2024

Yep that makes sense, let's see if I added it correctly.

Almost. :) You still need to add 3.11 to the build matrix in .github/workflows/tox.yml.

tox.ini Outdated
Comment on lines 6 to 7
3.8: py38-lilac,py38-maple,py38-nutmeg,py38-olive,py38-palm,py38-quince,py38-redwood,flake8
3.11: py311-lilac,py311-maple,py311-nutmeg,py311-olive,py311-palm,py311-quince,py311-redwood,flake8
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd argue that this ought to be sufficient:

Suggested change
3.8: py38-lilac,py38-maple,py38-nutmeg,py38-olive,py38-palm,py38-quince,py38-redwood,flake8
3.11: py311-lilac,py311-maple,py311-nutmeg,py311-olive,py311-palm,py311-quince,py311-redwood,flake8
3.8: py38-lilac,py38-maple,py38-nutmeg,py38-olive,py38-palm,py38-quince,flake8
3.11: py311-redwood,flake8

tox.ini Outdated
@@ -1,9 +1,10 @@
[tox]
envlist = py38-{lilac,maple,nutmeg,olive,palm,quince},flake8,report
envlist = py{38,311}-{lilac,maple,nutmeg,olive,palm,quince,redwood},flake8,report
Copy link
Contributor

Choose a reason for hiding this comment

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

... and we could possibly reduce the matrix here, too.

Suggested change
envlist = py{38,311}-{lilac,maple,nutmeg,olive,palm,quince,redwood},flake8,report
envlist = py38-{lilac,maple,nutmeg,olive,palm,quince},py311-redwood,flake8,report

Copy link
Contributor

@fghaas fghaas left a comment

Choose a reason for hiding this comment

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

Looks good to me now, thanks!

@mrtmm mrtmm merged commit c2e5523 into hastexo:main Aug 1, 2024
2 checks passed
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