Skip to content

Update to work with Django 4.0 and 4.1 #114

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

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

Conversation

BITSOLVER
Copy link

I forked the master from ierror/django-js-reverse and managed to get things working with Django 4.0 and 4.1 that I needed for a current project.

Hopefully this will be helpful to others.

@philippfreyer
Copy link

@BITSOLVER It seems like your PR is introducing a new dependency that is not explicitly listed - at least this is what I get when I try to use your forked release https://github.com/BITSOLVER/django-js-reverse/archive/refs/tags/0.10.1-b0.tar.gz as a dependency in my Django project:

Traceback (most recent call last):
  File "/srv/okmapgo/manage.py", line 21, in <module>
    main()
  File "/srv/okmapgo/manage.py", line 17, in main
    execute_from_command_line(sys.argv)
  File "/usr/local/lib/python3.9/site-packages/django/core/management/__init__.py", line 446, in execute_from_command_line
    utility.execute()
  File "/usr/local/lib/python3.9/site-packages/django/core/management/__init__.py", line 440, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/usr/local/lib/python3.9/site-packages/django/core/management/__init__.py", line 279, in fetch_command
    klass = load_command_class(app_name, subcommand)
  File "/usr/local/lib/python3.9/site-packages/django/core/management/__init__.py", line 48, in load_command_class
    module = import_module("%s.management.commands.%s" % (app_name, name))
  File "/usr/local/lib/python3.9/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1030, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
  File "<frozen importlib._bootstrap>", line 986, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 680, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 850, in exec_module
  File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
  File "/usr/local/lib/python3.9/site-packages/django_js_reverse/management/commands/collectstatic_js_reverse.py", line 10, in <module>
    from django_js_reverse.core import generate_js
  File "/usr/local/lib/python3.9/site-packages/django_js_reverse/core.py", line 7, in <module>
    from packaging.version import parse as LooseVersion
ModuleNotFoundError: No module named 'packaging'
Error response from daemon: The command '/bin/sh -c python manage.py collectstatic_js_reverse' returned a non-zero code: 1

Is your replacement of distutils.version.LooseVersion needed or a habit? In the first case, we could just revert the changes in django_js_reverse/core.py, in the latter we would need to add the requirement. (I say we, but I hope you do it so that I do not have to inject myself too much into this project as well - I love your other changes :-D )

@BITSOLVER
Copy link
Author

@philippfreyer As far as I can tell packaging is the preferred way to go in newer versions of python. I'll update the rest of tox.ini targets to include packaging. I'm a bit new at contributing to these apps on github so must of mist this. I'll see if I can update the pull request correctly!

@philippfreyer
Copy link

I am happy to hear a quick response - please check that setup.py also includes the reference then - this is what is evaluated when pulling the dependency through pip as far as I remember - I am not packaging these packages often as well ;-)
As soon as you push to your github branch that your PR is based on, your commit should also show up here. If you want to make me super happy, you would also rerelease the changed package on your repository since I am currently pulling the release from there - I could then remove my manual dependency to it already :-D

@BITSOLVER
Copy link
Author

@philippfreyer Ok that makes sense. I'm just investigation if I should make packaging conditional based on the python version and also the install_requires in setup.py conditional.

@BITSOLVER
Copy link
Author

@philippfreyer I've committed and pushed changes to my fork and created a prerelease.

@philippfreyer
Copy link

@BITSOLVER Thank you, this fixes the issues I had :-) - I just tried it in my build.

@hugobessa
Copy link

@BITSOLVER this repo is not being maintained anymore. The company I work on, Vinta Software, is the new maintainer now and has ability to launch new releases to django-js-reverse package in PyPI. We're going to do the updates on our fork of this repository.

Can I merge your changes here to our repo?

@BITSOLVER
Copy link
Author

BITSOLVER commented Aug 13, 2023 via email

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

3 participants