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

Some 2.0 release note tweaks #9310

Merged
merged 1 commit into from Nov 1, 2017

Conversation

adamchainz
Copy link
Sponsor Member

Just a few tweaks I noticed while reading through.

@@ -52,7 +52,7 @@ Simplified URL routing syntax
The new :func:`django.urls.path()` function allows a simpler, more readable URL
routing syntax. For example, this example from previous Django releases::

url(r'^articles/(?P<year>[0-9]{4})/$', views.year_archive),
url(r'^articles/(?P<year>[0-9]+)/$', views.year_archive),
Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

I found this confusing because just 'int' is used below which isn't bounded to 4 digits. I thought it best to make the regex url as close to the path.

Copy link
Member

Choose a reason for hiding this comment

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

I guess it'd be better to describe how the new syntax may be less restrictive.

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

I think changing the example to match is sufficient, it avoids distraction. We could maybe change the example entirely to use a model PK, which is a more common use case?

Copy link
Member

Choose a reason for hiding this comment

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

Well, I think it's an important consideration when switching to the new syntax. I'll make a proposal.

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

I agree it is, I just though the release note announcing the feature isn't necessarily the best place. Then again, maybe it is.

@@ -776,7 +776,8 @@ See :ref:`deprecated-features-1.10` for details on these changes.

* The ``shell --plain`` option is removed.

* The ``django.core.urlresolvers`` module is removed.
* The ``django.core.urlresolvers`` module is removed in favor of its new
Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

This one is kind of mentioned above in the "see deprecated features in 1.10" link, but it took me some time to notice when upgrading and I thought inlining it here would be helpful since I think it will be very common.

@@ -52,7 +52,7 @@ Simplified URL routing syntax
The new :func:`django.urls.path()` function allows a simpler, more readable URL
routing syntax. For example, this example from previous Django releases::

url(r'^articles/(?P<year>[0-9]{4})/$', views.year_archive),
url(r'^articles/(?P<year>[0-9]+)/$', views.year_archive),
Copy link
Member

Choose a reason for hiding this comment

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

I guess it'd be better to describe how the new syntax may be less restrictive.

@@ -96,7 +96,8 @@ Backwards compatibility
-----------------------

The following checks are performed to warn the user of any potential problems
that might occur as a result of a version upgrade.
that might occur as a result of a version upgrade. Some have been removed since
they were added as per the deprecation timeline.
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 sure if this will be helpful. Readers may not know what the deprecation timeline is and even if they do, the timeline doesn't discuss particulars of what checks are removed when. Maybe it's better just to remove old compatibility checks from the documentation when they're removed. I think the main reason to leave the docs for other removed checks is so that their code isn't reused. That's not an issue here since the code is based on the version number.

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

So, keep the heading and replace all content with a paragraph like "Sometimes checks are added for backwards compatibility between Django versions. At current there are no such checks, as they've been removed since they were no longer needed." ?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, done in f8946fb.

@timgraham timgraham merged commit 629dde8 into django:master Nov 1, 2017
@adamchainz adamchainz deleted the release_note_tweaks branch July 20, 2020 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants