We can switch this back in HEAD once the DRUPAL-6 branch opens. But we can't ship like this because people see a notice and think it's an error message. We did agree on this when we went for E_ALL compatibility.

CommentFileSizeAuthor
no_e_all_in_shipping_drupal.patch640 byteschx

Comments

chx’s picture

chx commented

Like http://drupal.org/cvs?commit=84985 .

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

We did ship every beta with E_NOTICE messages hidden, so do we plan to do with later betas and release candidates, whatever comes.

chx’s picture

chx commented
Status: Reviewed & tested by the community » Closed (fixed)

Then I remove this.

Crell’s picture

Crell commented
Status: Closed (fixed) » Active

Urk. I hadn't even noticed that in the betas.

So we're disabling E_NOTICE flags on shipping versions of Drupal, which means that module developers will be developing against a shipping version that doesn't have E_ALL properly set, so modules will end up not being E_ALL because their authors won't know they have E_NOTICE problems.

HOW is this a good thing, exactly?

chx’s picture

chx commented
Priority: Critical » Normal

I do not want to see this in the critical queue. Anyways, the problem here is that it's rather easy to make a notice-sized mistake, your code still works but when users see a big ugly error red message they will run screaming, no matter that's just a notice. And those notices can happen on rare cases and your code *still* works. So, forcing Drupal coders to write notice free code... I can't see that happen. Not to mention that even our best developers fscked up, not knowing when to use empty / isset.

Crell’s picture

Crell commented

That's what the "write errors to log but not show on screen" setting is for. Any production site should be running in that mode anyway, so there will be no big red notice on screen.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

Read this issue: http://drupal.org/node/99625#comment-471716

Crell’s picture

Crell commented

Yes, I read the issue. I still stand by my sentiment in it: Contrib authors will not be able to develop to E_ALL unless they are running E_ALL, even if they want to. Setting releases of core to !E_NOTICE tells developers (1) We think you're too lazy/stupid to develop to E_ALL standards and (2) "real developers"(tm) develop their modules against a -dev release of core so that they get E_ALL.

Remember, many servers run E_ALL and Drupal overriding that itself doesn't work. (I've run into a couple like that, including the one I use.) Even if core is E_ALL-compliant, one contrib module that is not will still cause nasty-red-messages on that server no matter what we set core to by default. That forces the site admin to go back to setting cryptic error codes in .htaccess, a process that is not at all documented. Making sure contrib authors write E_ALL code will avoid that problem, and various bugs to boot.

webchick pointed out http://drupal.org/node/100249 in the original issue as an example of a bug that shouldn't have been, had E_ALL been set. Why make contrib authors' lives (and the lives of people running their code) harder by not giving them that sort of debug information?

Anonymous’s picture

Anonymous (not verified) commented

Subscribing and weighing in my vote for E_ALL for all releases. The quicker the contrib module issues are brought to light the better it will be for everyone.

Crell’s picture

Crell commented
Title: Switch off E_ALL in shipping Drupal » Switch on E_ALL in shipping Drupal

Changing title to reflect the intent.

steven jones’s picture

steven jones commented
Status: Active » Closed (won't fix)

According to http://drupal.org/node/34341 we did turn on E_ALL reporting in D7, and I'm thinking that we don't want to induce massive pain by changing the way that D6 reports errors, so marking this issue as 'won't fix'.