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

Rename base_name => basename for consistency's sake #5990

Merged
merged 2 commits into from Jul 6, 2018

Conversation

Projects
None yet
4 participants
@rpkilby
Copy link
Member

rpkilby commented May 16, 2018

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.register needs 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 some commits May 16, 2018

@tomchristie

This comment has been minimized.

Copy link
Member

tomchristie commented May 21, 2018

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

@rpkilby

This comment has been minimized.

Copy link
Member Author

rpkilby commented May 21, 2018

Makes sense to me. Adding it to the milestone.

@rpkilby rpkilby added this to the 3.9 Release milestone May 21, 2018

@@ -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...

This comment has been minimized.

@carltongibson

carltongibson Jul 6, 2018

Collaborator

Why don't you like ?

This comment has been minimized.

@rpkilby

rpkilby Jul 6, 2018

Author Member

Oh - that was unintentional. I have a plugin that converts magic quotes to their ascii equivalent, but it also catches ellipses apparently.

@carltongibson
Copy link
Collaborator

carltongibson left a comment

OK. Let's have this. 👍

@carltongibson carltongibson merged commit 7095021 into encode:master Jul 6, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@rpkilby rpkilby deleted the rpkilby:basename branch Jul 6, 2018

@minusf

This comment has been minimized.

Copy link
Contributor

minusf commented Sep 14, 2018

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...

@carltongibson

This comment has been minimized.

Copy link
Collaborator

carltongibson commented Sep 14, 2018

@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.

@carltongibson

This comment has been minimized.

Copy link
Collaborator

carltongibson commented Sep 14, 2018

@minusf I've redeployed the docs. They look correct for v3.8 now. Thanks for pinging in the issue.

@minusf

This comment has been minimized.

Copy link
Contributor

minusf commented Sep 14, 2018

thanks for looking into this.

@antwan antwan referenced this pull request Dec 19, 2018

Open

DRF 3.9+ compatbility #26

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.