Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wordpress source normaliser plugin extension #3783

Closed
wants to merge 5 commits into from
Closed

Wordpress source normaliser plugin extension #3783

wants to merge 5 commits into from

Conversation

moshie
Copy link
Contributor

@moshie moshie commented Jan 30, 2018

This pull request references #3450

@gatsbybot
Copy link
Collaborator

Deploy preview for gatsbygram ready!

Built with commit 9caafa7

https://deploy-preview-3783--gatsbygram.netlify.com

@gatsbybot
Copy link
Collaborator

Deploy preview for gatsbygram ready!

Built with commit be7c293

https://deploy-preview-3783--gatsbygram.netlify.com

* @return {Array}
*/
getNormalizers() {
return _.concat(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use Array.concat() instead of lodash eliminates the need for extra dependency call in this file.

@KyleAMathews
Copy link
Contributor

Looks great! Curious why the need for explicit prioritization? Not my favorite way of doing things — ideally each normalization is completely independent of other normalizations so the order is irrelevant.

@KyleAMathews
Copy link
Contributor

If order matters, this often leads to confusing bugs.

@moshie
Copy link
Contributor Author

moshie commented Feb 2, 2018

I am not particularly fond of it myself tbh, only way I could think of doing it so a consuming plugin could be run before or after a certain built in normaliser.

Might be worth scraping the priority system if it's too bug prone?

@KyleAMathews
Copy link
Contributor

Maybe instead, there's a normalizer / WP entity and then in the options, you can override the core normalizer? Or follow it?

@KyleAMathews
Copy link
Contributor

Hey, would love a PR adding a simpler normalizer setup but going to close this for now.

This was referenced Sep 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants