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

Fixed #23891 -- Moved deprecation of IPAddressField to system check framework. #3818

Merged
merged 1 commit into from Jan 1, 2015

Conversation

timgraham
Copy link
Member

No description provided.

@MarkusH
Copy link
Member

MarkusH commented Dec 30, 2014

If we want to recommend this way for 3rd party apps as well, I suggest we offer a simple API, where e.g. setting a field class attribute will automatically raise a warning or error.

class Field(...):
    deprecated_for_migrations = False
    removed_for_migrations = False

    def check(...):
        # ...
        errors.extend(self._check_migration_availability(...))
        # ...

    def _check_migration_availability(self):
        if self.__class__.removed_for_migrations:
            return [
                checks.Error(
                    'The field %s has been removed except for support in historical migrations.' % self.__class__.__name__,
                    hint=None,
                    obj=self,
                    id='fields.EXXX',
                )
            ]
        elif self.__class__.deprecated_for_migrations:
            return [
                checks.Warning(
                    'The field %s has been deprecated. Support for it (except in historical migrations) will be removed according to the deprecation timeline' % self.__class__.__name__,
                    hint=None,
                    obj=self,
                    id='fields.WXXX',
                )
            ]

class IPAddressField(...):
    deprecated_for_migrations = True

@MarkusH
Copy link
Member

MarkusH commented Dec 30, 2014

The check id should be added to docs/ref/checks.txt

@timgraham
Copy link
Member Author

Markus, can we make your suggestion the subject of ticket #23861 then? That seems like a new feature and I am not sure it requires a backport to 1.7.

I wasn't sure about the WXXX codes which is why I didn't document it yet. It seems nice to be able to have a unique code for each field so they can be silenced if desired, rather than one code for all deprecations, but this seems possible only if we use the model field class name to generate the code, in which case we'll have to depart from the usual numbering scheme.

@MarkusH
Copy link
Member

MarkusH commented Dec 31, 2014

I missed the fact you want to backport this to 1.7. I that case, yes, this could be part of #23861 if it makes sense.

@timgraham
Copy link
Member Author

Markus, updated #23891 per your comments and added a sketch for how #23861 might look. Will merge the former and continue work on the latter if all looks okay..

return [
checks.Warning(
self.deprecated_for_migrations.get(
'message', '%s has been deprecated.' % self.__class__.__name__
Copy link
Member

Choose a reason for hiding this comment

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

Could you wrap this line after the comma.

@MarkusH
Copy link
Member

MarkusH commented Jan 1, 2015

Looks good to me, Tim.

…ramework.

Thanks Markus Holtermann for review.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants