Re: Please Apply #5801 - GET parameters are ignored in redirect for Admin
Errr, isn't that a duplicate of #5775? http://code.djangoproject.com/ticket/5775 On Jul 19, 6:19 am, Rozza <[EMAIL PROTECTED]> wrote: > New patch with tests added to the ticket. > > Shows the reason for the triage process! > > All the best > > Ross > > On Jul 18, 2:54 pm, Rozza <[EMAIL PROTECTED]> wrote: > > > Thanks Russell, > > > I write the tests and attach to the ticket asap. > > > Ross > > > On Jul 18, 2:01 pm, "Russell Keith-Magee" <[EMAIL PROTECTED]> > > wrote: > > > > On Fri, Jul 18, 2008 at 8:47 PM, Rozza <[EMAIL PROTECTED]> wrote: > > > > > Ah no problems! > > > > > As there aren't any tests for the staff_member_required or even > > > > request.get_full_path() I wasn't 100% sure what the procedure would be > > > > as these are used through out Django. > > > > > As it's such a minor change does it warrant the bureaucracy of > > > > requiring tests to have it implemented? > > > > Yes. Next question? :-) > > > > There are some parts of Django that either do not have tests. These > > > usually correspond to the oldest parts of the framework, which existed > > > before Django had a full testing framework. Tha'ts not an excuse to > > > avoid writing tests - it just means that your tests will be the first > > > tests in that area. > > > > The tests serve multiple purposes: > > > - they demonstrate the existence of a problem, which makes the triage > > > process easier > > > - they demonstrate that your patch fixes the problem > > > - they prevent the problem from happening again in the future. > > > > With very few exceptions, nothing gets committed to the code base > > > unless it has tests. > > > > > I think definitely there should be a ticket opened to ensure that > > > > tests are written to test the logic staff_member_required and > > > > request.get_full_path(). At the moment I'm not sure where they would > > > > fit. > > > > The hard part is writing the tests, not deciding where to put them. > > > Worst case, the core developer that commits your patch will put the > > > tests into a different location. > > > > However, that said, the decision on where to put a test goes something > > > like this: > > > * Is it a specific test of a contrib app? If yes, put it in the tests > > > module for that app > > > * Could the test serve as a form of documentation for a feature? If > > > yes, put it in modeltests. > > > * Otherwise, put it in regressiontests. > > > > Beyond that, try to add the tests to an existing test module within > > > modeltests/regressiontests; however, if you can't find anything > > > appropriate, feel free to suggest a new test module. > > > > Yours, > > > Russ Magee %-) --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~--~~~~--~~--~--~---
Re: Please Apply #5801 - GET parameters are ignored in redirect for Admin
Yeah looks like it except #5775 just focuses on the decorator and #5801 focuses on all admin logins (the original patch just looked at the decorator though). The new patch for #5801 focuses both on the decorated custom admin views and normal admin views. It also normalises the behaviour between the decorator logic and the sites.py login method as they behaved differently. Ross On Jul 18, 11:07 pm, Julien Phalip <[EMAIL PROTECTED]> wrote: > Errr, isn't that a duplicate of > #5775?http://code.djangoproject.com/ticket/5775 > > On Jul 19, 6:19 am, Rozza <[EMAIL PROTECTED]> wrote: > > > New patch with tests added to the ticket. > > > Shows the reason for the triage process! > > > All the best > > > Ross > > > On Jul 18, 2:54 pm, Rozza <[EMAIL PROTECTED]> wrote: > > > > Thanks Russell, > > > > I write the tests and attach to the ticket asap. > > > > Ross > > > > On Jul 18, 2:01 pm, "Russell Keith-Magee" <[EMAIL PROTECTED]> > > > wrote: > > > > > On Fri, Jul 18, 2008 at 8:47 PM, Rozza <[EMAIL PROTECTED]> wrote: > > > > > > Ah no problems! > > > > > > As there aren't any tests for the staff_member_required or even > > > > > request.get_full_path() I wasn't 100% sure what the procedure would be > > > > > as these are used through out Django. > > > > > > As it's such a minor change does it warrant the bureaucracy of > > > > > requiring tests to have it implemented? > > > > > Yes. Next question? :-) > > > > > There are some parts of Django that either do not have tests. These > > > > usually correspond to the oldest parts of the framework, which existed > > > > before Django had a full testing framework. Tha'ts not an excuse to > > > > avoid writing tests - it just means that your tests will be the first > > > > tests in that area. > > > > > The tests serve multiple purposes: > > > > - they demonstrate the existence of a problem, which makes the triage > > > > process easier > > > > - they demonstrate that your patch fixes the problem > > > > - they prevent the problem from happening again in the future. > > > > > With very few exceptions, nothing gets committed to the code base > > > > unless it has tests. > > > > > > I think definitely there should be a ticket opened to ensure that > > > > > tests are written to test the logic staff_member_required and > > > > > request.get_full_path(). At the moment I'm not sure where they would > > > > > fit. > > > > > The hard part is writing the tests, not deciding where to put them. > > > > Worst case, the core developer that commits your patch will put the > > > > tests into a different location. > > > > > However, that said, the decision on where to put a test goes something > > > > like this: > > > > * Is it a specific test of a contrib app? If yes, put it in the tests > > > > module for that app > > > > * Could the test serve as a form of documentation for a feature? If > > > > yes, put it in modeltests. > > > > * Otherwise, put it in regressiontests. > > > > > Beyond that, try to add the tests to an existing test module within > > > > modeltests/regressiontests; however, if you can't find anything > > > > appropriate, feel free to suggest a new test module. > > > > > Yours, > > > > Russ Magee %-) --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~--~~~~--~~--~--~---
Re: Please Apply #5801 - GET parameters are ignored in redirect for Admin
New patch with tests added to the ticket. Shows the reason for the triage process! All the best Ross On Jul 18, 2:54 pm, Rozza <[EMAIL PROTECTED]> wrote: > Thanks Russell, > > I write the tests and attach to the ticket asap. > > Ross > > On Jul 18, 2:01 pm, "Russell Keith-Magee" <[EMAIL PROTECTED]> > wrote: > > > On Fri, Jul 18, 2008 at 8:47 PM, Rozza <[EMAIL PROTECTED]> wrote: > > > > Ah no problems! > > > > As there aren't any tests for the staff_member_required or even > > > request.get_full_path() I wasn't 100% sure what the procedure would be > > > as these are used through out Django. > > > > As it's such a minor change does it warrant the bureaucracy of > > > requiring tests to have it implemented? > > > Yes. Next question? :-) > > > There are some parts of Django that either do not have tests. These > > usually correspond to the oldest parts of the framework, which existed > > before Django had a full testing framework. Tha'ts not an excuse to > > avoid writing tests - it just means that your tests will be the first > > tests in that area. > > > The tests serve multiple purposes: > > - they demonstrate the existence of a problem, which makes the triage > > process easier > > - they demonstrate that your patch fixes the problem > > - they prevent the problem from happening again in the future. > > > With very few exceptions, nothing gets committed to the code base > > unless it has tests. > > > > I think definitely there should be a ticket opened to ensure that > > > tests are written to test the logic staff_member_required and > > > request.get_full_path(). At the moment I'm not sure where they would > > > fit. > > > The hard part is writing the tests, not deciding where to put them. > > Worst case, the core developer that commits your patch will put the > > tests into a different location. > > > However, that said, the decision on where to put a test goes something > > like this: > > * Is it a specific test of a contrib app? If yes, put it in the tests > > module for that app > > * Could the test serve as a form of documentation for a feature? If > > yes, put it in modeltests. > > * Otherwise, put it in regressiontests. > > > Beyond that, try to add the tests to an existing test module within > > modeltests/regressiontests; however, if you can't find anything > > appropriate, feel free to suggest a new test module. > > > Yours, > > Russ Magee %-) --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~--~~~~--~~--~--~---
Re: Please Apply #5801 - GET parameters are ignored in redirect for Admin
On Fri, Jul 18, 2008 at 8:47 PM, Rozza <[EMAIL PROTECTED]> wrote: > > Ah no problems! > > As there aren't any tests for the staff_member_required or even > request.get_full_path() I wasn't 100% sure what the procedure would be > as these are used through out Django. > > As it's such a minor change does it warrant the bureaucracy of > requiring tests to have it implemented? Yes. Next question? :-) There are some parts of Django that either do not have tests. These usually correspond to the oldest parts of the framework, which existed before Django had a full testing framework. Tha'ts not an excuse to avoid writing tests - it just means that your tests will be the first tests in that area. The tests serve multiple purposes: - they demonstrate the existence of a problem, which makes the triage process easier - they demonstrate that your patch fixes the problem - they prevent the problem from happening again in the future. With very few exceptions, nothing gets committed to the code base unless it has tests. > I think definitely there should be a ticket opened to ensure that > tests are written to test the logic staff_member_required and > request.get_full_path(). At the moment I'm not sure where they would > fit. The hard part is writing the tests, not deciding where to put them. Worst case, the core developer that commits your patch will put the tests into a different location. However, that said, the decision on where to put a test goes something like this: * Is it a specific test of a contrib app? If yes, put it in the tests module for that app * Could the test serve as a form of documentation for a feature? If yes, put it in modeltests. * Otherwise, put it in regressiontests. Beyond that, try to add the tests to an existing test module within modeltests/regressiontests; however, if you can't find anything appropriate, feel free to suggest a new test module. Yours, Russ Magee %-) --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~--~~~~--~~--~--~---
Re: Please Apply #5801 - GET parameters are ignored in redirect for Admin
Thanks Russell, I write the tests and attach to the ticket asap. Ross On Jul 18, 2:01 pm, "Russell Keith-Magee" <[EMAIL PROTECTED]> wrote: > On Fri, Jul 18, 2008 at 8:47 PM, Rozza <[EMAIL PROTECTED]> wrote: > > > Ah no problems! > > > As there aren't any tests for the staff_member_required or even > > request.get_full_path() I wasn't 100% sure what the procedure would be > > as these are used through out Django. > > > As it's such a minor change does it warrant the bureaucracy of > > requiring tests to have it implemented? > > Yes. Next question? :-) > > There are some parts of Django that either do not have tests. These > usually correspond to the oldest parts of the framework, which existed > before Django had a full testing framework. Tha'ts not an excuse to > avoid writing tests - it just means that your tests will be the first > tests in that area. > > The tests serve multiple purposes: > - they demonstrate the existence of a problem, which makes the triage > process easier > - they demonstrate that your patch fixes the problem > - they prevent the problem from happening again in the future. > > With very few exceptions, nothing gets committed to the code base > unless it has tests. > > > I think definitely there should be a ticket opened to ensure that > > tests are written to test the logic staff_member_required and > > request.get_full_path(). At the moment I'm not sure where they would > > fit. > > The hard part is writing the tests, not deciding where to put them. > Worst case, the core developer that commits your patch will put the > tests into a different location. > > However, that said, the decision on where to put a test goes something > like this: > * Is it a specific test of a contrib app? If yes, put it in the tests > module for that app > * Could the test serve as a form of documentation for a feature? If > yes, put it in modeltests. > * Otherwise, put it in regressiontests. > > Beyond that, try to add the tests to an existing test module within > modeltests/regressiontests; however, if you can't find anything > appropriate, feel free to suggest a new test module. > > Yours, > Russ Magee %-) --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~--~~~~--~~--~--~---
Re: Please Apply #5801 - GET parameters are ignored in redirect for Admin
Ah no problems! As there aren't any tests for the staff_member_required or even request.get_full_path() I wasn't 100% sure what the procedure would be as these are used through out Django. As it's such a minor change does it warrant the bureaucracy of requiring tests to have it implemented? I think definitely there should be a ticket opened to ensure that tests are written to test the logic staff_member_required and request.get_full_path(). At the moment I'm not sure where they would fit. Happy to help either way. Ross On Jul 18, 9:04 am, "Russell Keith-Magee" <[EMAIL PROTECTED]> wrote: > On Fri, Jul 18, 2008 at 3:57 PM, Rozza <[EMAIL PROTECTED]> wrote: > > > Hi all, > > > This is a bump for ticket #5801 it was marked as accepted back in > > February but has not yet been applied! > > > Apologies if this is against protocol - it just seems this simple > > patch has been lost amongst the many! > > If you look at the triage process [1], you will see that there is a > good reason that the patch hasn't been applied. Accepted means that we > acknowledge that the bug is real, not that the patch is ready. A patch > will only be applied when the ticket has been marked ready for > checkin. > > The obvious reason for this ticket not being ready for checkin is that > the patch doesn't have any tests. This should be a relatively simple > case to check with the built in test client, so there is no excuse for > not having tests. Write some tests, and we'll get this ticket > committed. > > [1]http://www.djangoproject.com/documentation/contributing/#ticket-triage > > Yours, > Russ Magee %-) --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~--~~~~--~~--~--~---
Re: Please Apply #5801 - GET parameters are ignored in redirect for Admin
On Fri, Jul 18, 2008 at 3:57 PM, Rozza <[EMAIL PROTECTED]> wrote: > > Hi all, > > This is a bump for ticket #5801 it was marked as accepted back in > February but has not yet been applied! > > Apologies if this is against protocol - it just seems this simple > patch has been lost amongst the many! If you look at the triage process [1], you will see that there is a good reason that the patch hasn't been applied. Accepted means that we acknowledge that the bug is real, not that the patch is ready. A patch will only be applied when the ticket has been marked ready for checkin. The obvious reason for this ticket not being ready for checkin is that the patch doesn't have any tests. This should be a relatively simple case to check with the built in test client, so there is no excuse for not having tests. Write some tests, and we'll get this ticket committed. [1] http://www.djangoproject.com/documentation/contributing/#ticket-triage Yours, Russ Magee %-) --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~--~~~~--~~--~--~---
Please Apply #5801 - GET parameters are ignored in redirect for Admin
Hi all, This is a bump for ticket #5801 it was marked as accepted back in February but has not yet been applied! Apologies if this is against protocol - it just seems this simple patch has been lost amongst the many! Ross --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~--~~~~--~~--~--~---
