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

Supress some protected-access warnings when django-simple-history is installed. #423

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

m000
Copy link

@m000 m000 commented Dec 12, 2023

django-simple-hisory is a fairly popular [1] package for keeping track of changes in django objects. Setting the _change_reason property of an object is the officially documented way to provide a value for the history_change_reason field of historical objects [2].

When django-simple-hisory is installed, protected-access warnings for setting _change_reason is most likely a false positive, and should be supressed.

Because of inherent limitations of pylint, this may lead to some false negatives if _change_reason is used elsewhere.

[1] https://pypistats.org/packages/django-simple-history
[2] https://django-simple-history.readthedocs.io/en/latest/historical_model.html#change-reason

@m000
Copy link
Author

m000 commented Dec 12, 2023

I know this is missing a test. I would first like to bring it up for discussion/feedback before adding one.

I am working on a codebase that has 24 cases (and counting) where the specific warning is suppressed for _change_reason. For this, I think this would make a meaningful addition for pylint-django.

…installed.

django-simple-hisory is a fairly popular [1] package for keeping track
of changes in django objects. Setting the `_change_reason` property of
an object is the officially documented way to provide a value for the
`history_change_reason` field of historical objects [2].

When django-simple-hisory is installed, protected-access warnings for
setting `_change_reason` is most likely a false positive, and should be
supressed.

Because of inherent limitations of pylint, this may lead to some false
negatives if `_change_reason` is used elsewhere.

[1] https://pypistats.org/packages/django-simple-history
[2] https://django-simple-history.readthedocs.io/en/latest/historical_model.html#change-reason
@m000 m000 force-pushed the simple_history_change_reason branch from 655ebf2 to 438151f Compare December 12, 2023 16:46
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.

1 participant