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.0, Remove support to older Django and Python versions and improvements #35

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

mrsarm
Copy link

@mrsarm mrsarm commented Feb 21, 2022

  • Made fixes to make it work with Django 4.0.
  • Drop support to Python 2 and Python < 3.5 that are unmaintained and insecure.
  • Drop support to Django < 2.2 that also reach end of mainstream support (insecure).
  • Drop included jQuery version used in favor of built-in django.jQuery in Django that is more up to date and maintained by Django, making also the library less vulnerable and lightweight.
  • Fix margin in "chips", specially margin top was 0, making it ugly.
  • Fix Tox and Selenium configurations. Replace PhantomJS with Chrome (PhantomJS support was removed in newer versions of Selenium).
  • Replace Travis CI with GitHub Actions: Travis is fading out its commitment with OSS, making the service for no-paid projects each time slower and less available. On the other hand GH Actions is free and fast even for OSS projects.
  • Remove IDE configurations that are "user" related.

@mrsarm mrsarm changed the title Remove support to older Django and Python versions and improvements Django 4Remove support to older Django and Python versions and improvements Feb 21, 2022
@mrsarm mrsarm changed the title Django 4Remove support to older Django and Python versions and improvements Django 4.0, Remove support to older Django and Python versions and improvements Feb 21, 2022
@mrsarm
Copy link
Author

mrsarm commented Feb 21, 2022

I didn't stress enough that the main change was adding support to Django 4.0 that was broken (although the easiest part) , so I changed the title of the PR and the description to reflect that.

* Made fixes to make it work with Django 4.0.
* Drop support to Python 2 and Python < 3.5 that are unmaintained and insecure.
* Drop support to Django < 2.2 that also reach end of mainstream support (insecure).
* Drop included jQuery version used in favor of built-in `django.jQuery` in Django that is more up to date and maintained by Django, making also the library less vulnerable.
* Fix margin in "chips", specially margin top was 0, making it ugly.
* Fix Tox and Selenium configurations. Replace PhantomJS with Chrome (PhantomJS support was removed in newer versions of Selenium).
* Replace Travis CI with GitHub Actions: Travis is fading out its commitment with OSS, making the service for no-paid projects each time slower and less available. On the other hand GH Actions is free and fast even for OSS projects.
* Remove IDE configurations that are "user" related.
@mrsarm
Copy link
Author

mrsarm commented Feb 21, 2022

And here are the tests results on Github Actions, that are not easily visible here I think because the changes are in my repo / branch and not in this repo yet: https://github.com/mrsarm/django-searchable-select/actions/runs/1876330131.

@FriedrichFroebel
Copy link

I just had a quick look at the CI stuff, without further diving into your actual changes.

  • Why are you using Python 3.5 and Python 3.10 only? I understand that these should probably be the supported version range, but I do not think that it will hurt to test the versions in-between as well, as the duration is short enough and this should not require many changes.

  • If you are adding compatibility for the latest and supported Django versions, we should probably fix the existing system check warnings. From a quick look, there seems to be at least one type:

    2022-02-21T13:12:26.8469138Z example.Cat: (models.W042) Auto-created primary key used when not defining a primary key type, by default 'django.db.models.AutoField'.
    2022-02-21T13:12:26.8470013Z 	HINT: Configure the DEFAULT_AUTO_FIELD setting or the AppConfig.default_auto_field attribute to point to a subclass of AutoField, e.g. 'django.db.models.BigAutoField'.
    2022-02-21T13:12:26.8470862Z example.Food: (models.W042) Auto-created primary key used when not defining a primary key type, by default 'django.db.models.AutoField'.
    2022-02-21T13:12:26.8471571Z 	HINT: Configure the DEFAULT_AUTO_FIELD setting or the AppConfig.default_auto_field attribute to point to a subclass of AutoField, e.g. 'django.db.models.BigAutoField'.
    2022-02-21T13:12:26.8472256Z example.Person: (models.W042) Auto-created primary key used when not defining a primary key type, by default 'django.db.models.AutoField'.
    2022-02-21T13:12:26.8472985Z 	HINT: Configure the DEFAULT_AUTO_FIELD setting or the AppConfig.default_auto_field attribute to point to a subclass of AutoField, e.g. 'django.db.models.BigAutoField'.
    

Regarding the CI results not being visible here: According to the docs, GitHub applies the workflow definitions from the base branch.

Note: Workflows triggered by pull_request_target events are run in the context of the base branch. Since the base branch is considered trusted, workflows triggered by these events will always run, regardless of approval settings.

@mrsarm
Copy link
Author

mrsarm commented Feb 21, 2022

Hi @FriedrichFroebel , thanks for your feedback ! You are right, I added more Python versions and still run fast enough, and added also Django 3.0 that was missed in the matrix.

Also fixed the warnings in Django 3.2+.

Check the last execution at https://github.com/mrsarm/django-searchable-select/actions/runs/1876512153.

@FriedrichFroebel
Copy link

Another thing which just popped up for me are the names like py35-22 which seems to indicate Python 3.5 running Django 2.2. I am not sure whether these should be made more obvious/descriptive.

@mrsarm
Copy link
Author

mrsarm commented Feb 21, 2022

What do you propose to name the items? Maybe we can add the dj prefix to the Django versions, so the matrix would look like:

envlist =
    py35-{dj22}
    py36-{dj30,dj31,dj32}
    ...

And Python 3.8 with Django 3.2 would be py38-dj32.

@FriedrichFroebel
Copy link

The dj prefix (or even python and django as the longer versions) would probably be enough for me to improve readability.

@mrsarm
Copy link
Author

mrsarm commented Feb 21, 2022

Done ! Now an item from the matrix execution looks like: py3.8-dj3.0.

@mrsarm
Copy link
Author

mrsarm commented Feb 23, 2022

Hello Mr @and3rson ! I made these changes, and I see you are looking for developers to take care of the project.

Do you want to review the changes and ultimately merge and release a new version? Or do you want me take care of that? If you give me the rights in the repo and in PyPI I'm happy to take care of it. BTW my username in PyPI is the same than Github: https://pypi.org/user/mrsarm/

I also working in a few more changes to make the widget more flexible, and allow for instance to populate an ArrayField with values from another table.

Not latest version of each library though, because compatibility issues with older versions of Python.
The rst version has the same content and is well rendered by Github
@mrsarm
Copy link
Author

mrsarm commented Feb 24, 2022

I created in a separated PR a new feature that allows to use the plugin with the ArrayField type, so the base branch is the branch from this same PR, and I didn't include the changes here to not make the PR hard to review with so many changes.

In the meantime, anyone that needs to use this widget on Django 4, or wants to use it with array fields, take a look to the other PR, where there is also instructions of how to use it installing the package from GitHub instead of PyPI: mrsarm#1

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