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.

Sign up for GitHub

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

Jump to bottom

Fixed #28766 -- Added ResolverMatch.route #10657

Merged
merged 1 commit into from
Dec 6, 2018
Merged

Fixed #28766 -- Added ResolverMatch.route #10657

merged 1 commit into from
Dec 6, 2018

Conversation

xavfernandez
Copy link
Contributor

@xavfernandez xavfernandez commented Nov 17, 2018 •
edited by timgraham
Loading

@xavfernandez xavfernandez mentioned this pull request Nov 17, 2018
Closed
@xavfernandez xavfernandez force-pushed the xfernandez/recursive_routes branch from 2a81dde to 95b1b68 Compare November 17, 2018 12:33
auvipy
auvipy reviewed Nov 17, 2018
View reviewed changes
docs/ref/urlresolvers.txt Outdated
@@ -130,6 +130,12 @@ If the URL does not resolve, the function raises a

The name of the URL pattern that matches the URL.

.. attribute:: ResolverMatch.route

.. versionadded:: 2.1
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this should be 2.2?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, and please add a release note in docs/releases/2.2.txt.

docs/ref/urlresolvers.txt Outdated

.. versionadded:: 2.1

The route of the URL pattern that matches the URL.

This comment was marked as resolved.

Sign in to view

@timgraham
Copy link
Member

Please check test coverage carefully. I don't see any failures if the change in ResolverMatch.__repr__() is reverted, for example. Uncheck "patch needs improvement" on the ticket when it's ready for review again.

@xavfernandez xavfernandez force-pushed the xfernandez/recursive_routes branch from 3de38c4 to 3aa89e5 Compare November 19, 2018 11:46
@xavfernandez xavfernandez reopened this Nov 19, 2018
@xavfernandez xavfernandez force-pushed the xfernandez/recursive_routes branch 2 times, most recently from adae3eb to b758f4d Compare November 21, 2018 16:34
@xavfernandez
Copy link
Contributor Author

Hello @timgraham, sorry to nag but is there anything missing or something I must/could do to help this get merged ?

@timgraham timgraham force-pushed the xfernandez/recursive_routes branch from 4438f2d to e5a0332 Compare November 30, 2018 01:28
timgraham
timgraham reviewed Nov 30, 2018
View reviewed changes
django/urls/resolvers.py Outdated Show resolved Hide resolved
tests/urlpatterns/path_urls.py Outdated Show resolved Hide resolved
@xavfernandez xavfernandez force-pushed the xfernandez/recursive_routes branch from 4b0a248 to c8d81c2 Compare November 30, 2018 15:52
@timgraham timgraham force-pushed the xfernandez/recursive_routes branch from c8d81c2 to 14fd32f Compare November 30, 2018 23:07
@beniwohli @xavfernandez @timgraham
Fixed #28766 -- Added ResolverMatch.route.
79c196c 
Co-Authored-By: Xavier Fernandez <xavier.fernandez@polyconseil.fr>
@timgraham timgraham force-pushed the xfernandez/recursive_routes branch from 14fd32f to 79c196c Compare December 6, 2018 23:06
@timgraham timgraham merged commit 79c196c into django:master Dec 6, 2018
@xavfernandez xavfernandez deleted the xfernandez/recursive_routes branch December 7, 2018 09:30
@xavfernandez
Copy link
Contributor Author

Thanks @timgraham for the guidance :)

@cristianmedeiros cristianmedeiros mentioned this pull request Jan 30, 2019
Closed
adamantike added a commit to adamantike/opentelemetry-python-contrib that referenced this pull request Oct 26, 2021
@adamantike
django: Fix instrumentation and tests for all Django major versions
ef95233 
* Fix support for Django `1.10`/`1.11`, by allowing `settings.MIDDLEWARE`.
* Add Tox environments for every Django major version, including 4.0 beta.
* Fix tests for new Tox environments to pass.

Relevant context:

* In Django 1.x, usage of `settings.MIDDLEWARE_CLASSES` is detected by
  running `settings.MIDDLEWARE is None`:
  https://github.com/django/django/blob/58f02c498b659a906e9c30d946bd89bedc4717e5/django/core/handlers/base.py#L48
* HTTP route in `ResolverMatch` object is only available since Django 2.2:
  django/django#10657
* Django class `HttpResponseBase` works as a mapping object, so avoiding
  accessing its internal headers object simplifies support accross
  Django versions.
@adamantike adamantike mentioned this pull request Oct 26, 2021
Merged
7 tasks
adamantike added a commit to adamantike/opentelemetry-python-contrib that referenced this pull request Oct 28, 2021
@adamantike
django: Fix instrumentation and tests for all Django major versions
f29172c 
* Fix support for Django `1.10`/`1.11`, by allowing `settings.MIDDLEWARE`.
* Add Tox environments for every Django major version, including 4.0 beta.
* Fix tests for new Tox environments to pass.

Relevant context:

* In Django 1.x, usage of `settings.MIDDLEWARE_CLASSES` is detected by
  running `settings.MIDDLEWARE is None`:
  https://github.com/django/django/blob/58f02c498b659a906e9c30d946bd89bedc4717e5/django/core/handlers/base.py#L48
* HTTP route in `ResolverMatch` object is only available since Django 2.2:
  django/django#10657
* Django class `HttpResponseBase` works as a mapping object, so avoiding
  accessing its internal headers object simplifies support accross
  Django versions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timgraham timgraham timgraham left review comments

@auvipy auvipy auvipy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@xavfernandez @timgraham @auvipy @beniwohli