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

Django 4.2/DjangoCMS 3.11 support #58

Closed
mogoh opened this issue Jun 26, 2023 · 7 comments
Closed

Django 4.2/DjangoCMS 3.11 support #58

mogoh opened this issue Jun 26, 2023 · 7 comments
Assignees

Comments

@mogoh
Copy link

mogoh commented Jun 26, 2023

If I ran python manage.py makemigrations after updating to Django 4.2/DjangoCMS 3.11, new migrations will be generated.

@marksweb
Copy link
Sponsor Member

Would be best with updates based on django-cms/djangocms-audio#32

@mogoh
Copy link
Author

mogoh commented Jul 3, 2023

Some questions:

  • Are the changes in github actions/ci required?
  • How do we use pyproject.toml and tox.ini? Is the usage documented? Can someone point me in the right direction?
  • djangocms-audio uses some custom tool to generate test requirements. Do we need this or can we ignore it for now?

On the one hand, I understand the wish to modernize the pipeline.
On the other hand, I just want a new version with up to date migrations.
Can we decouple this a bit?

@marksweb
Copy link
Sponsor Member

marksweb commented Jul 3, 2023

@mogoh You're looking at classic maintainer vs user problems.

I want packages to be easy maintain and reliable so that when we release something, we know it's going to work. You just want it to be available and work.

In terms of the changes I've made to djangocms-audio, some are related to the way we manage packages. We look to have consistency across all our packages and to make things as easy as possible for everyone. Part of that is using conventional commits which is why we lint PR titles. For linting itself we recently moved to use ruff in place of flake8 and isort.

Changes to the test workflow mean that github actions just call tox which is what you'd do as a developer to run the test suite. So the change there is consistency between CI and development tests. Using pre-commit ensures consistency and standards are met in all code that is committed.

The use of pyproject.toml has/is becoming the most common way to configure packages/tools and generally the documentation for this is in the tools that need the configuration.

The requirements compile script gives us an improved way to maintain requirements for testing because it automatically generates the requirements files, with hashes. We just have to input a little info about python version and packages that are meant for the particular test setup. These also feed into the tox workflow for testing.

Then finally the tox defines the tests we run and you'll see that the test env names match up to the requirements file names. This means installing the requirements for that test are as simple as;

[testenv]
deps =
    -r tests/requirements/{envname}.txt

Because that was my PR to solve your issue, I did the whole lot. But really, you could submit PRs to include the migration and the corresponding tests passing. Then we could release that.

@marksweb marksweb self-assigned this Jul 3, 2023
@marksweb
Copy link
Sponsor Member

marksweb commented Jul 3, 2023

@mogoh Could you confirm the migration that was created please?

I'm unable to create a new migration using django 4.2.3.

djangocms-file on  overhaul [!?] via 🐍 on ☁️ 
❯ django-admin --version
4.2.3

djangocms-file on  overhaul [!?] via 🐍 on ☁️  
❯ python -m manage makemigrations djangocms_file
No changes detected in app 'djangocms_file'

@mogoh
Copy link
Author

mogoh commented Jul 5, 2023

Thank you for your in depth explanation!
I am looking into it, but I might take some time.

@mogoh
Copy link
Author

mogoh commented Jul 5, 2023

@mogoh Could you confirm the migration that was created please?

I'm unable to create a new migration using django 4.2.3.

djangocms-file on  overhaul [!?] via 🐍 on ☁️ 
❯ django-admin --version
4.2.3

djangocms-file on  overhaul [!?] via 🐍 on ☁️  
❯ python -m manage makemigrations djangocms_file
No changes detected in app 'djangocms_file'

As far as I can tell, everything is looking good.
I still don't fully understand everything, but it looks plausible.
Thank you for your work!

@marksweb
Copy link
Sponsor Member

marksweb commented Jul 5, 2023

@mogoh thanks for confirming 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants