afragen / local-development

WordPress plugin to provide local development message for selected plugins/themes
GNU General Public License v2.0
24 stars 7 forks source link

keep vcs icon in the "same" position #8

Closed Raruto closed 4 years ago

Raruto commented 4 years ago

Minor edit, in order to avoid that the vcs icon is added after the "in local development" message (easier to see and understand)

Raruto commented 4 years ago

oops, what a mess!

why do we still see all the comments from the previous one?

afragen commented 4 years ago

oops, what a mess!

why do we still see all the comments from the previous one?

I have no idea.

I'm not certain as to this PR. All it seems to do is move the conditional and filter higher in the load_hooks() function. I'm not sure this is what you had in mind. Also a screenshot would be nice.

afragen commented 4 years ago

Do you mean like this?

screenshot_22
Raruto commented 4 years ago

All it seems to do is move the conditional and filter higher in the load_hooks() function ... Do you mean like this?

Yep, in this way they still share the same hook priority (15).

afragen commented 4 years ago

I've got a fix locally that explicitly ensures the hook priority is correct regardless of when the hook is called.

Raruto commented 4 years ago

however in your way https://github.com/afragen/local-development/commit/016e1d15dd9bdd0dd5d21fe57ce00c06a1c0f8f0#diff-e69ddc6ae9435975d1d746184610655e you cannot always be sure that they will always be side by side ( eg. external hook call with priority 17 )

afragen commented 4 years ago

True, but we can't count on every possible interaction of other plugins with out code.

Raruto commented 4 years ago

yes, but since the execution of the code is sequential, what sense does it make to introduce this "randomness"? (wouldn't the code be even cleaner to read?)

afragen commented 4 years ago

I’ll change the priority to be sequential. That should be less random. 😉

Are we ready to release?

Raruto commented 4 years ago

Are we ready to release?

👌