djangocms-attributes-field icon indicating copy to clipboard operation
djangocms-attributes-field copied to clipboard

Delete deprecated argument

Open jfalfaro opened this issue 3 years ago • 6 comments

Support for context argument was removed in Django 3.0

jfalfaro avatar Nov 01 '21 19:11 jfalfaro

@heller166 Thanks for contributing, Can you please write this code in a backward compatible way so that it doesn't break for old version of Django and works for the newer versions too?

vinitkumar avatar Nov 01 '21 19:11 vinitkumar

I'm not sure that I can do that now that. I was trying to avoid getting a RemovedInDjango30Warning that is issued in the following code. The function that is executed to check if context is present checks the method signature and I can't come up with a way for it to be removed while keeping backwards compatibility.

def func_supports_parameter(func, parameter):
    return parameter in inspect.signature(func).parameters

jfalfaro avatar Nov 01 '21 20:11 jfalfaro

Codecov Report

Merging #42 (0076a64) into master (d55e448) will not change coverage. The diff coverage is n/a.

:exclamation: Current head 0076a64 differs from pull request most recent head 5c257d9. Consider uploading reports for the commit 5c257d9 to get more accurate results Impacted file tree graph

@@           Coverage Diff           @@
##           master      #42   +/-   ##
=======================================
  Coverage   90.25%   90.25%           
=======================================
  Files           3        3           
  Lines         154      154           
  Branches       26       26           
=======================================
  Hits          139      139           
  Misses         10       10           
  Partials        5        5           
Impacted Files Coverage Δ
djangocms_attributes_field/fields.py 87.17% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d55e448...5c257d9. Read the comment docs.

codecov-commenter avatar Nov 02 '21 04:11 codecov-commenter

@heller166 You did't had to close your pull request.

Check this code, how we can fix some deprecation warnings or errors using this conditionals, and also this:

from platform import python_version
from django import get_version

from distutils.version import LooseVersion


DJANGO_VERSION = get_version()
PYTHON_VERSION = python_version()

# These means "less than or equal to DJANGO_FOO_BAR"
DJANGO_2_2 = LooseVersion(DJANGO_VERSION) < LooseVersion('3.0')
DJANGO_3_0 = LooseVersion(DJANGO_VERSION) < LooseVersion('3.1')
DJANGO_3_1 = LooseVersion(DJANGO_VERSION) < LooseVersion('3.2')
DJANGO_3_2 = LooseVersion('3.2') <= LooseVersion(DJANGO_VERSION) and  LooseVersion(DJANGO_VERSION) < LooseVersion('3.3')

https://github.com/django-cms/django-cms/pull/7062/commits/cc394d9c2845eeff87d1350820c4cbb33157ba97#diff-5452cbe13e8c4195320b722d63a033d4670ecd0d57a3ba6772d1fd1152fd4d58L40

vinitkumar avatar Nov 02 '21 04:11 vinitkumar

@vinitkumar thanks for the suggestion. I implemented a solution, let me know what you think. I tried to come up with some way to test this but it seemed a bit more trouble than its worth.

jfalfaro avatar Nov 02 '21 19:11 jfalfaro

@vinitkumar looks like there has been some work since your comments so could you review again?

marksweb avatar Mar 27 '22 20:03 marksweb