Rename base_name => basename for consistency's sake by rpkilby · Pull Request #5990 · encode/django-rest-framework (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation9 Commits2 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
The viewsets and routers use both base_name and basename. It would be nice if this were consistent.
I've deprecated base_name in favor of basename, and get_default_base_name in favor of get_default_basename. Fortunately, the deprecation is fairly straightforward.
Router.registerneeds to handle both arguments appropriately (fallback to the old value if present, raise deprecation warnings, complain if both arguments are provided)- Django has a deprecation utility metaclass that already handles method renames.
Ryan P Kilby added 2 commits
Seems fair enough, yup.
Given that we work against master we typically don't pull in deprecation changes until we're confident that the next release will be one of the major version bumps. I guess we could leave this on hold until we're getting ready to release 3.9
Makes sense to me. Adding it to the milestone.
| @@ -65,7 +65,7 @@ For example, you can append `router.urls` to a list of existing views… |
|---|
| urlpatterns += router.urls |
| Alternatively you can use Django's `include` function, like so… |
| Alternatively you can use Django's `include` function, like so... |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't you like …?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh - that was unintentional. I have a plugin that converts magic quotes to their ascii equivalent, but it also catches ellipses apparently.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. Let's have this. 👍
This is very confusing at the moment. The online docs already talk only about basename while the last PyPi released version expects base_name.
I understand that the online docs are not versioned like the django ones, but I think in that case they should be either clearly marked as DEV, or they should follow the latest stable release, used in production environments...
@minusf Yes. That's not great. A recent docs roll-out will have updated this. I'll see if I can patch-up something correct for the time before v3.9.
@minusf I've redeployed the docs. They look correct for v3.8 now. Thanks for pinging in the issue.
thanks for looking into this.
ScriptSmith added a commit to eresearchqut/trrf that referenced this pull request
kmohrf pushed a commit to grouprise/grouprise that referenced this pull request
djangorestframework v3.9 deprecated the keyword argument "base_name" for routers. The old name works up to djangorestframework 3.10, but fails later.
See encode/django-rest-framework#5990
pchiquet pushed a commit to pchiquet/django-rest-framework that referenced this pull request
Rename base_name => basename for consistency
Update tests to use basename
slamora added a commit to ribaguifi/django-orchestra that referenced this pull request
jsugarman added a commit to ministryofjustice/laa-fee-calculator that referenced this pull request
jsugarman added a commit to ministryofjustice/laa-fee-calculator that referenced this pull request