buddypress / next-template-packs

is this the next BuddyPress template pack?
35 stars 9 forks source link

"forsaken" action/filter feature. #196

Closed paulgibbs closed 6 years ago

paulgibbs commented 7 years ago

@hnla I think this needs to be changed, or removed. Not because it's a bad idea, but because

1) the browser console is not where I expect to see PHP deprecation messages. 2) this adds a huge amount of translatable strings which are very technical and niche.

Ideas, from easy to hard:

hnla commented 7 years ago

Hmm ok, wasn't aware these presented a problem. originally added by imath so not sure the best approach forward - removal sounds the best path given the comments but not sure how 'fragile' the code is to that.

hnla commented 7 years ago

Re-factors the messages away from the localized_scripts function to a new function to pass the message array to the WP debug.php file.

Removes the i18n function to ease the burden on translators, these are dev messages and essentially English is the international language for programming/development so devs will need to suffer - mea culpa!

N.b Messages print to file only not screen, but this could be changed?

r-a-y commented 7 years ago

Regarding commit 696519e and printing the deprecated hooks into debug.log, I think we shouldn't do this.

My debug.log is currently being filled with Nouveau's deprecated hooks all the time! I actually preferred the JS console for this.

If we do decide to keep these notices, we need a filter to allow devs to disable this.

hnla commented 7 years ago

Can you two argue it out then, I'm sort of disinterested in my time being spent on things like this that don't move the real aim of the work forward - frankly should have just deleted the whole darned thing.

N.B I do appreciate the comments though @r-a-y, just tired and grisly :(

hnla commented 7 years ago

Commenting out the add_action to prevent log file abuse.

I suggest that alt approach might be to add an admin customizer option to turn on Nouveau debug notices, then inject an html element via JS and populate a discrete screen display informing devs of issues.

paulgibbs commented 7 years ago

let's just pull this out

hnla commented 6 years ago

Closing - needs checking in core, on Trac.