Some functions seems to be deprecated, therefore some dsm() have been put in order to warn if called: this code has been commited and if it runs on production, it might cause WSOD because dsm() won't exist without devel module: maybe triggering a watchdog notice or such could be better?

See ctools.module lines 681 and 689.

Files: 
CommentFileSizeAuthor
#5 1313382-dsm-remove-5.patch2.68 KBandypost
PASSED: [[SimpleTest]]: [MySQL] 51 pass(es).
[ View ]
#3 1313382-dsm-change2debug-3.patch2.53 KBandypost
PASSED: [[SimpleTest]]: [MySQL] 51 pass(es).
[ View ]

Comments

Status:Active» Closed (won't fix)

This bug report has not been active for six months. In an effort to clean-up the issue queue this item has been closed. If your modules are current and the report is still relevant please feel free to change the Status back to active.

Status:Closed (won't fix)» Needs work

Only a few places needs review

ctools# grep -r "dsm" ./
./ctools.module:  dsm('should not be called!');
./ctools.module:  dsm('should not be called!');
./page_manager/page_manager.module:      dsm($retval);
./page_manager/page_manager.admin.inc:        dsm($path);
./page_manager/page_manager.admin.inc:        dsm($item);
./page_manager/page_manager.admin.inc:        dsm($attributes['class']);
./includes/stylizer.inc://  dsm($processor->message_log);

Status:Needs work» Needs review
StatusFileSize
new2.53 KB
PASSED: [[SimpleTest]]: [MySQL] 51 pass(es).
[ View ]

Status:Needs review» Needs work

Why keep any of this? I'd say remove it completely.

Status:Needs work» Needs review
StatusFileSize
new2.68 KB
PASSED: [[SimpleTest]]: [MySQL] 51 pass(es).
[ View ]

Suppose this commented out code is a thing of a past.
Suppose we need to ping someone from ctools team to carefully review, probably this clean-up could be deeper.

can we get this commited?

Code that should not be called may throw logic error exception, to explicitely tell the code he did a bad mistake: finding errors right away forces the devs to fix them. I'm just sayin', I really don't know why you kept those functions in the first place, there's maybe a good reason.

It was a bad idea of me to use dsm() to point out these errors, but pounard is right -- it's better for developers to get explicit errors.

I don't remember why those functions are still there at this point. Possibly worth doing a little code archeaology to see.

so... let's change them to trigger_error()?

Throw named exceptions.

Depends on how is fatal the error, but if they are, exceptions are the way to go.

Status:Needs review» Fixed

Okay, I went through the list. I removed the no longer needed context_cache functions. I replaced a dsm about $attributes['class'] with a correction to just make it an array. (Experience has shown that there's Always One More 'class' setting that was not updated to the array notation).

I left in the commented out dsm()s. In a way, they're a reminder of something I had intended but haven't gotten back to. Maybe someday I or someone will, and the commented code at least says something about it. Removing it guarantees nothing will ever happen there.

Committed and pushed.

Status:Fixed» Closed (fixed)

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