Large mega patches re: E_ALL notices (Re: [drupal-devel] I do not care about PHP 5

Gabor Hojtsy gabor at hojtsy.hu
Sun Oct 16 10:57:16 UTC 2005


> Part of the patch reads like this:
> 
>  function error_handler($errno, $message, $filename, $line) 
> {
> -  if ($errno & (E_ALL ^ E_NOTICE)) {
> +  if ($errno & (E_ALL )) {
> 
> This is the heart of the matter! but changing this line will 
> suddenly affect all the code, because it is currently quite 
> messy, and error notices will start to crop up everywhere, 
> making more than one person unhappy. So the patch would 
> need to be BIIIG so as to cover at the very least 90% of 
> possible errors (one single page view of admin/settings 
> printed no less than 1480 errors and maybe more than 1500 
> on the screen... all dealt with by the patch I propose!!!)

The solution is to fix the problems in different smaller to test/review
patches, and then once all errors are fixed, apply this E_ALL change
(which might not be a good idea for contrib module users, but that is a
different question).

If you provide smaller patches, they are more likely to come into the
code, because they are easier to review, and it is easier to find the
free time slot to do so. It is also easier to discuss one type of
change, which you applied to the whole Drupal, then several issues at
once. Also it is easier to introduce the coding guideline which comes
out of the acceptance (or rejection) of the smaller patch.

> The solution would be to NOT change that line, but then, 
> other developpers will not see the error notices and would 
> carry on with their bad coding practices while we try to 
> keep up providing patches, hoping they get committed.
> What's the point in creating disparate small patches while 
> the codes changes rapidly and more mistakes (undeclared 
> variables) are made than we get the opportunity to fix?

Update the coding guildelines, and with the accepted small patches,
Dries et. al. will get the message that they should look for conformance
to the updated guidelines in future accepted patches. This approach
worked before.

Keep up the good work. I myself very much appreciate the work done to
make Drupal E_ALL compliant, but I can only find the time to look into
and review smaller patches, and I am sure there are numerous others like me.

Goba



More information about the drupal-devel mailing list