Summary

Remove the app_name for integration URLs.

Review Request #10472 — Created March 27, 2019 and submitted April 10, 2019, 8:29 p.m.

Information

Owner

chipx86

Repository

Djblets

Branch

release-2.0.x

Bugs

Depends On

Reviewers

Groups

djblets

People

Description

Django 1.6 happily allows an application namespaec (app_name) to be

specified for any URLs, but newer 1.x versions only allow this if an

instance namespace (namespace) is being provided. Django 2.x doesn't

allow the parameter at all, instead requiring a whole different process

for specifying the app namespace.
This change removes the application namespace entirely for integration

URLs. We don't use it to resolve URLs ourselves, and callers can always

provide their own namespace, so there's not much point in keeping this

around.

Testing Done

Djblets and Review Board unit tests pass on Django 1.6 and 1.11.

Commits

Summary	ID
Remove the app_name for integration URLs. Django 1.6 happily allows an application namespaec (`app_name`) to be specified for any URLs, but newer 1.x versions only allow this if an instance namespace (`namespace`) is being provided. Django 2.x doesn't allow the parameter at all, instead requiring a whole different process for specifying the app namespace. This change removes the application namespace entirely for integration URLs. We don't use it to resolve URLs ourselves, and callers can always provide their own namespace, so there's not much point in keeping this around.	4ca45556cef596c28ee3dc9da9bfa2b0d4958afd

Summary

Remove the app_name for integration URLs.

Django 1.6 happily allows an application namespaec (`app_name`) to be specified for any URLs, but newer 1.x versions only allow this if an instance namespace (`namespace`) is being provided. Django 2.x doesn't allow the parameter at all, instead requiring a whole different process for specifying the app namespace. This change removes the application namespace entirely for integration URLs. We don't use it to resolve URLs ourselves, and callers can always provide their own namespace, so there's not much point in keeping this around.

4ca45556cef596c28ee3dc9da9bfa2b0d4958afd

Issues

Description	From	Last Updated
Hmm, actually, this comment is confusing (seems backwards to the code).	david	April 10, 2019, 4:30 p.m.

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

djblets/integrations/urls.py (Diff revision 1)

The issue has been resolved. Show all issues

Hmm, actually, this comment is confusing (seems backwards to the code).

chipx86 March 28, 2019, 5:53 p.m.

It's the right way. If we're going to set an app_name, a namespace must be provided. Namespaces, however, do not require an app_name.

david April 4, 2019, 11:03 a.m.

I'm just saying the way it's written makes either the comment or the code seem wrong. How about "Django requires that we specify either both a namespace and app_name or neither."

chipx86 April 10, 2019, 4:37 p.m.

Turns out the app_name parameter to include is deprecated, and they have a new method for dealing with app names that Django 1.6 doesn't support. Sooo I'm just going to remove this entirely :) We don't really make use of it anywhere.

Change Summary:

Reworked this completely to just eliminate the app_name parameter, for future compatibility with Django 2.0.

Summary:

Only specify an app_name for integration URLs when using namespaces.

Remove the app_name for integration URLs.

Description:

~		Django 1.6 happily allows an `app_name` parameter to be specified for
~		any URLs, but newer versions only allow this if a namespace is being
~		provided. This change accounts for this, passing one only if the caller
~		has a specific namespace they want to use.
	~	Django 1.6 happily allows an application namespaec (`app_name`) to be
	~	specified for any URLs, but newer 1.x versions only allow this if an
	~	instance namespace (`namespace`) is being provided. Django 2.x doesn't
	~	allow the parameter at all, instead requiring a whole different process
	+	for specifying the app namespace.
	+
	+	This change removes the application namespace entirely for integration
	+	URLs. We don't use it to resolve URLs ourselves, and callers can always
	+	provide their own namespace, so there's not much point in keeping this
	+	around.

Commits:

	Summary	ID
	Only specify an app_name for integration URLs when using namespaces. Django 1.6 happily allows an `app_name` parameter to be specified for any URLs, but newer versions only allow this if a namespace is being provided. This change accounts for this, passing one only if the caller has a specific namespace they want to use.	d07359a3fb07a70a65a61ce0209edfc76f9a7187
	Remove the app_name for integration URLs. Django 1.6 happily allows an application namespaec (`app_name`) to be specified for any URLs, but newer 1.x versions only allow this if an instance namespace (`namespace`) is being provided. Django 2.x doesn't allow the parameter at all, instead requiring a whole different process for specifying the app namespace. This change removes the application namespace entirely for integration URLs. We don't use it to resolve URLs ourselves, and callers can always provide their own namespace, so there's not much point in keeping this around.	4ca45556cef596c28ee3dc9da9bfa2b0d4958afd

Diff:

Revision 2 (+2 -4)

Show changes

djblets/integrations/urls.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Status:: Completed
Change Summary:: Pushed to release-2.0.x (7241bdc)