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

Update to Django 2.0 Routing Syntax #6049

Merged
merged 3 commits into from
Jun 22, 2018
Merged

Update to Django 2.0 Routing Syntax #6049

merged 3 commits into from
Jun 22, 2018

Conversation

chrisshyi
Copy link
Contributor

Updated the code in tutorial part 6, "Using Routers", to Django 2.0
routing syntax using path(). Refs #5964, there was one missing bit that wasn't updated.

Updated the code in tutorial part 6, "Using Routers", to Django 2.0
routing syntax using path().
Copy link
Contributor

@blueyed blueyed left a comment

Choose a reason for hiding this comment

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

Thanks, but I think the newline should be kept.

@@ -3,7 +3,6 @@
REST framework includes an abstraction for dealing with `ViewSets`, that allows the developer to concentrate on modeling the state and interactions of the API, and leave the URL construction to be handled automatically, based on common conventions.

`ViewSet` classes are almost the same thing as `View` classes, except that they provide operations such as `read`, or `update`, and not method handlers such as `get` or `put`.

Copy link
Contributor

Choose a reason for hiding this comment

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

why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I erased that line by accident, will add it back in. Thanks!

@@ -116,7 +116,7 @@ Here's our re-wired `snippets/urls.py` file.

# The API URLs are now determined automatically by the router.
urlpatterns = [
url(r'^', include(router.urls))
path('', include(router.urls)),
Copy link
Contributor

Choose a reason for hiding this comment

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

path should also be imported then instead of url - sorry for missing this earlier.

You can add [ci skip] to not trigger a CI run.

Copy link
Contributor Author

@chrisshyi chrisshyi Jun 21, 2018

Choose a reason for hiding this comment

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

My bad, where would I add the [ci skip]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Damn, just realized I should've included it in the commit message, already committed...

Copy link
Member

Choose a reason for hiding this comment

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

I'm not 100% sure, but I don't think non-members/collaborators can issue a [ci skip].

Fixed the import statement in tutorial part 6 for the path() and
include() functions.
Copy link
Contributor

@blueyed blueyed left a comment

Choose a reason for hiding this comment

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

Thanks for your attention to details.

Please squash-merge this then.

@chrisshyi
Copy link
Contributor Author

Thanks for reviewing my work!

@carltongibson carltongibson merged commit d9f5418 into encode:master Jun 22, 2018
@carltongibson carltongibson added this to the 3.8.3 Release milestone Jun 22, 2018
@rpkilby rpkilby modified the milestones: 3.8.3 Release, 3.9 Release Aug 29, 2018
pchiquet pushed a commit to pchiquet/django-rest-framework that referenced this pull request Nov 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants