Quantcast

RE: Minor bug fix for users with no roles

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RE: Minor bug fix for users with no roles

McBride, Ian S.
Attached is a patch with all the strict mode fixes based off a copy of HEAD I downloaded today.

-----Original Message-----
From: McBride, Ian [mailto:[hidden email]]
Sent: Monday, June 15, 2009 11:47 AM
To: Monster Menus Development
Subject: RE: Minor bug fix for users with no roles

True enough. I actually went through and fixed them all in one of the early copies of MM we received, but then I updated to your latest dev copy on Friday. I'll work on compiling a patch of just these as I come across them again and post that to the list when it's ready.

-----Original Message-----
From: Dan Wilga [mailto:[hidden email]]
Sent: Monday, June 15, 2009 11:45 AM
To: Monster Menus Development
Subject: RE: Minor bug fix for users with no roles

At 11:23 AM -0400 6/15/09, McBride, Ian wrote:
>That's correct, the script will still work since it will evaluate
>false, but PHP will generate a Notice when it encounters an
>uninitialized variable in a conditional check, which I want to get
>rid of if possible to make logging cleaner.

Ah, OK, you have strict checking turned on. There are probably going
to be lots (easily dozens) of places where this happens, then. It
would be best to fix them all in one patch. Have fun! :-)
--
Dan Wilga                                 [hidden email]
Web System Administrator/Programmer             http://www.amherst.edu
Amherst College                                      Tel: 413-542-2175
Amherst, MA  01002

---
You are currently subscribed to monster_menus as: [hidden email].
To unsubscribe click here: http://lists.middlebury.edu/u?id=685439.7e7cbccf9bb225cf8471bffe1cb67503&n=T&l=monster_menus&o=424546
or send a blank email to [hidden email]

---
You are currently subscribed to monster_menus as: [hidden email].
To unsubscribe click here: http://lists.middlebury.edu/u?id=685439.7e7cbccf9bb225cf8471bffe1cb67503&n=T&l=monster_menus&o=424547
or send a blank email to [hidden email]

---
You are currently subscribed to monster_menus as: [hidden email].
To unsubscribe click here: http://lists.middlebury.edu/u?id=685503.6b071f880fe6a965a128164e6d09ea81&n=T&l=monster_menus&o=446155
or send a blank email to [hidden email]

monster_menus.20090903.patch (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RE: Minor bug fix for users with no roles

Dan Wilga-2
Hi Ian,

This is all set, in r3129. I made one minor change:

       $cache[$path]->node = isset($node) ? $node : NULL;

I used NULL here, instead of ''.

I'm actually surprised there were so few of these. I'll bet you get a
lot more you're just not bothering to fix, because they are so
infrequent :-). Personally, I just run with:

   error_reporting  =  E_ALL & ~E_NOTICE & ~E_WARNING

in php.ini, as most of what is reported by E_WARNING is of the "I'll
help keep you from wondering why your code doesn't run as expected at
the expense of making you do a lot of extra work" variety.
--
Dan Wilga                                 [hidden email]
Web System Administrator/Programmer             http://www.amherst.edu
Amherst College                                      Tel: 413-542-2175
Amherst, MA  01002

---
You are currently subscribed to monster_menus as: [hidden email].
To unsubscribe click here: http://lists.middlebury.edu/u?id=685503.6b071f880fe6a965a128164e6d09ea81&n=T&l=monster_menus&o=446168
or send a blank email to [hidden email]
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Minor bug fix for users with no roles

Adam Franco
Administrator
In reply to this post by McBride, Ian S.
Thanks for the fix Dan and Ian!

I have a different philosophical take on notices I just wanted to throw out there for consideration.

While I certainly agree with Dan that fixing notices and warnings tend one toward more verbose code, I would just like to throw my vote behind programming using error_reporting = E_ALL | E_STRICT and running production with E_ERROR (and display_errors off).

Competent developers and good style-guides generally result in decent code whatever the situation, but way too many times I've had to debug something written by an intern (or myself when I was an intern) that would have been obvious had they been showing all notices while programming. One of the common ones I've fixed several times is a conditional always evaluating to false because one of the arguments wasn't initialized in a switch statement. An even more common one is just the simple misspelled variable name (especially for me -- I'm a poor speller).

Turning on notices and warnings certainly doesn't solve all problems -- and there are a few valid ways of programming that will always raise notices -- but on the whole I'd estimate that warnings and notices have saved me more time that they've wasted.

Thanks for reading, my apologies if this seemed pedantic. I'll shut up now. :-)
Adam


On Mon, Aug 3, 2009 at 11:42 AM, Dan Wilga <[hidden email]> wrote:
Hi Ian,

This is all set, in r3129. I made one minor change:

     $cache[$path]->node = isset($node) ? $node : NULL;

I used NULL here, instead of ''.

I'm actually surprised there were so few of these. I'll bet you get a lot more you're just not bothering to fix, because they are so infrequent :-). Personally, I just run with:

 error_reporting  =  E_ALL & ~E_NOTICE & ~E_WARNING

in php.ini, as most of what is reported by E_WARNING is of the "I'll help keep you from wondering why your code doesn't run as expected at the expense of making you do a lot of extra work" variety.
--
Dan Wilga                                 [hidden email]
Web System Administrator/Programmer             http://www.amherst.edu
Amherst College                                      Tel: 413-542-2175
Amherst, MA  01002

---
You are currently subscribed to monster_menus as: [hidden email].
To unsubscribe click here: http://lists.middlebury.edu/u?id=685438.780c6126d238396bdd2f98c1d84c15c7&n=T&l=monster_menus&o=446168
or send a blank email to [hidden email]

---

You are currently subscribed to monster_menus as: [hidden email].

To unsubscribe click here: http://lists.middlebury.edu/u?id=685503.6b071f880fe6a965a128164e6d09ea81&n=T&l=monster_menus&o=446228

(It may be necessary to cut and paste the above URL if the line is broken)

or send a blank email to [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Minor bug fix for users with no roles

Dan Wilga-2
In reply to this post by McBride, Ian S.
At 2:21 PM -0400 8/3/09, Adam Franco wrote:
>While I certainly agree with Dan that fixing notices and warnings
>tend one toward more verbose code, I would just like to throw my
>vote behind programming using error_reporting = E_ALL | E_STRICT and
>running production with E_ERROR (and display_errors off).

You're probably in the majority here. I think that, as we move more
toward generalizing the code, making it strict-compliant is probably
a good idea. I wish I could say when my time will be available for
this part of the project, but at this point I'm booked solid until at
least October.

--
Dan Wilga                                 [hidden email]
Web System Administrator/Programmer             http://www.amherst.edu
Amherst College                                      Tel: 413-542-2175
Amherst, MA  01002

---
You are currently subscribed to monster_menus as: [hidden email].
To unsubscribe click here: http://lists.middlebury.edu/u?id=685503.6b071f880fe6a965a128164e6d09ea81&n=T&l=monster_menus&o=446299
or send a blank email to [hidden email]
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Minor bug fix for users with no roles

Adam Franco
Administrator
In reply to this post by McBride, Ian S.
No worries on the time-line. I'm hoping that this will be something we can help out with (Such as with Ian's patch). I imagine that we will also be able to help out with some kinds of documentation as we try to figure things out with MM.

Regards,
Adam

On Mon, Aug 3, 2009 at 4:50 PM, Dan Wilga <[hidden email]> wrote:
At 2:21 PM -0400 8/3/09, Adam Franco wrote:
While I certainly agree with Dan that fixing notices and warnings tend one toward more verbose code, I would just like to throw my vote behind programming using error_reporting = E_ALL | E_STRICT and running production with E_ERROR (and display_errors off).

You're probably in the majority here. I think that, as we move more toward generalizing the code, making it strict-compliant is probably a good idea. I wish I could say when my time will be available for this part of the project, but at this point I'm booked solid until at least October.


--
Dan Wilga                                 [hidden email]
Web System Administrator/Programmer             http://www.amherst.edu
Amherst College                                      Tel: 413-542-2175
Amherst, MA  01002

---
You are currently subscribed to monster_menus as: [hidden email].
To unsubscribe click here: http://lists.middlebury.edu/u?id=685438.780c6126d238396bdd2f98c1d84c15c7&n=T&l=monster_menus&o=446299
or send a blank email to [hidden email]

---

You are currently subscribed to monster_menus as: [hidden email].

To unsubscribe click here: http://lists.middlebury.edu/u?id=685503.6b071f880fe6a965a128164e6d09ea81&n=T&l=monster_menus&o=446303

(It may be necessary to cut and paste the above URL if the line is broken)

or send a blank email to [hidden email]

Loading...