Hi John, It's a bit unfortunate that I can't find any trace of your changes. They are probably on your local machine, so I can't see any of the changes you have made or the errors they cause.
Do you believe the problems you are running into come from a bug in the path() implementation, not yet fully understanding it, or something else? Also, I think the changes to all the other tests URLs is a "nice to have" but not necessarily a merge-blocker. In fact, letting the old syntax remain tested is still important. Some more remarks inline. Regards, Sjoerd Job On Tuesday, August 1, 2017 at 5:40:58 PM UTC+2, John Griebel wrote: > > Hi Sjoerd, Emil (and everyone else), > I wanted to give a quick update here so you all have some visibility into > the progress being made. I have begun by working on the "Update the tests > throughout, updating to the new style wherever possible." in Sjoerd's check > list just to get my feet wet and familiarize myself with the code base a > little more, how path() works, etc. > > At this point I've converted the URL configs for all test packages with > the following exceptions: > 1.) auth_tests - About 16 tests are still failing, all related to > password-reset. > It would be great to have some info on the failure: an exception, error, ? > 2.) check_framework - path() makes some URLs configurations valid that are > not valid with url() such as including '$' in an include, etc. For the time > being, I have elected not to convert the URLs that would be valid with > path() but not url(). I am sure this merits some discussion. If it's > already been discussed and I missed it, just let me know. > In the path() syntax, a dollar sign is interpreted as matching a literal dollar sign, not as in "end of string". > 3.) generic_views - 16 tests are failing, all related to date tests. I > suspect the majority of problems here will be fixed by getting urls.py > exactly correct. I do think there may have to be a small change to the > _date_from_string method in /generic/dates.py, however. > would love to see the test failures. > 4.) i18n - I haven't dealt much with i18n in the past so I've left this > one for a bit later. > > 5.) messages_test - Only one failure that I need to figure out. > Kindly share with us. > 6.) urlpatterns_reverse - Should these be left as is, and similar tests > for path() be created since url() will not be deprecated immediately? > in urlpatterns_reverse I have left most testing as-is, because I think it is important still for re_path (url) to be validated. Should probably be left untouched. > 7.) view_tests - I see Sjoerd already made some small changes here and > wanted to confirm that the rest of the URL conf was not left unconverted > intentionally before converting them myself. > > Feedback on 6 and 7, in particular, would be great. > > Thanks, > John Griebel | Senior Backend Engineer | 3Blades.io | 814-227-4213 > > On Sat, Jul 29, 2017 at 10:01 AM, John Griebel <johnkg...@gmail.com > <javascript:>> wrote: > >> I think the review comments and the Todo list should get me off to a good >> start. I'm sure I'll have a few questions along the way as I've contributed >> to other open source projects before, but not Django. I spent some time >> last evening reading the contribution docs and getting my environment set >> up. >> >> >> On Jul 29, 2017 4:49 AM, "Emil Stenström" <e...@kth.se <javascript:>> >> wrote: >> >> John: Awesome! Do you need anything to get started or is Sjoerd's review >> comments and todolist enough? >> >> >> On Friday, 28 July 2017 15:03:12 UTC+2, John Griebel wrote: >> >>> Hi Sjoerd, >>> I too would love to see this feature in Django 2.0; in fact, I've been >>> following its progress quite closely. I would love to help out with it and >>> have the necessary time to do so. >>> >>> John Griebel | Senior Backend Engineer | 3Blades.io | 814-227-4213 >>> >>> On Thu, Jul 27, 2017 at 5:06 PM, Sjoerd Job Postmus <sjoe...@sjec.nl> >>> wrote: >>> >>>> Hi all, >>>> >>>> Due to scheduling, I have not been able to give this the attention I >>>> would like to give it, and I don't see myself freeing up a significant >>>> amount of time to give it the final push forward before the feature freeze >>>> on September 18th. >>>> >>>> I did a final review of my changes. Though there are some things that >>>> might need a bit of cleaning up, I have the feeling the pull request ( >>>> https://github.com/django/django/pull/7482) is in a reasonable state. >>>> There are a lot of comments by me about things that might be improved, >>>> though. There's also a TODO list ( >>>> https://github.com/django/django/pull/7482#issuecomment-305482442) >>>> about things that still need doing. >>>> >>>> Based on the check-list, I think the following items remain: >>>> >>>> - Documentation the new syntax (but Tom already has a PR open for >>>> that), but that should only be picked up after this feature is >>>> merged/mergeable. >>>> - Updating existing documentation (maybe even the tutorial?) to use the >>>> new syntax. I think that's also included in Tom's PR. >>>> - Extra tests. There are dedicated tests already, but given such a core >>>> feature I would feel more comfortable if there were even more tests. >>>> - Not mentioned in the checklist, but performance concerns should also >>>> be checked [1]. >>>> >>>> Personally, I would love for this feature to land in Django 2.0, but >>>> I'm doubtful that I can push it further myself, and would be greatly >>>> appreciative if somebody were willing to help push it forward. >>>> >>>> [1]: Previously, the code used `LocaleRegexDescriptor`, which acted as >>>> `@property` when the regular expression was lazy, while if it is a pure >>>> string it would set the attribute directly as an optimisation trick. This >>>> might need to be replicated. >>>> >>>> Regards, >>>> Sjoerd Job >>>> >>>> -- >>>> You received this message because you are subscribed to the Google >>>> Groups "Django developers (Contributions to Django itself)" group. >>>> To unsubscribe from this group and stop receiving emails from it, send >>>> an email to django-develop...@googlegroups.com. >>>> To post to this group, send email to django-d...@googlegroups.com. >>>> >>>> Visit this group at https://groups.google.com/group/django-developers. >>>> To view this discussion on the web visit >>>> https://groups.google.com/d/msgid/django-developers/d75698e7-e75e-4719-a000-4172f3ed1082%40googlegroups.com >>>> >>>> <https://groups.google.com/d/msgid/django-developers/d75698e7-e75e-4719-a000-4172f3ed1082%40googlegroups.com?utm_medium=email&utm_source=footer> >>>> . >>>> For more options, visit https://groups.google.com/d/optout. >>>> >>> >>> -- >> You received this message because you are subscribed to the Google Groups >> "Django developers (Contributions to Django itself)" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to django-develop...@googlegroups.com <javascript:>. >> To post to this group, send email to django-d...@googlegroups.com >> <javascript:>. >> Visit this group at https://groups.google.com/group/django-developers. >> To view this discussion on the web visit >> https://groups.google.com/d/msgid/django-developers/12ac3285-5d09-4fe1-bbf2-b34c75150724%40googlegroups.com >> >> <https://groups.google.com/d/msgid/django-developers/12ac3285-5d09-4fe1-bbf2-b34c75150724%40googlegroups.com?utm_medium=email&utm_source=footer> >> . >> >> For more options, visit https://groups.google.com/d/optout. >> >> >> > -- You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscr...@googlegroups.com. To post to this group, send email to django-developers@googlegroups.com. Visit this group at https://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/27229da2-7535-4b43-bb33-7056b341071e%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.
