Hi,

I have found an E_NOTICE when I save a node. I attached the patch which solves the problem.

Comments

tamasd’s picture

StatusFileSize
new865 bytes

another one patch

danielb’s picture

Status: Active » Postponed

The first patch will break the behaviour of this module in certain situations because it will set the values to FALSE/0 if they've already been set to TRUE/1 by another field (someone may have a field for 'viewers', and a field for 'authors', for example). For E_ALL compliance the least damaging change to make is turn if ($var) into if (isset($var)).
In the 2nd patch there is now a double check for isset and is_array which could be simplified by using a !empty instead.

Don't worry about making a patch. If and when I set out to achieve E_ALL compliance I can go through everything by hand.

danielb’s picture

Category: bug » feature

And I would really put E_ALL compliance as a feature since I've only recently started hearing talk about drupal being compliant. The sloppy if tests I use are often taken right out of core of popular contrib modules.

Coornail’s picture

I think it's an entirely wrong method to handle E_ALL compilance patches.

From the error_reporting php manual site:

Enabling E_NOTICE during development has some benefits. For debugging purposes: NOTICE messages will warn you about possible bugs in your code. For example, use of unassigned values is warned. It is extremely useful to find typos and to save time for debugging. NOTICE messages will warn you about bad style.

These errors refering to bad programming habits, and mostly easy to fix.
(E_NOTICEs can easily changed to E_WARNINGs with a php version update, so it's good for forward compatibility also).

danielb’s picture

I have no idea what you're saying coornail.

Forgive my ignorance, I am interested in algorithms, data structures, and providing functionality to users. The nuances of how servers are configured and PHP error reporting has generally been the server administrator's problem and something I'm not allowed to touch. I frankly don't know much about E_ANYTHING so I will have to get back to this issue after I've educated myself a bit more.

danielb’s picture

Status: Postponed » Fixed

Have added some changes that should take care of the problem.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.