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

Use U.S. English spelling for 'optimization'. #11584

Closed
wants to merge 2 commits into from

Conversation

davidangel
Copy link
Contributor

@davidangel davidangel commented Oct 20, 2017

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@orlangur orlangur self-assigned this Oct 20, 2017
Copy link
Contributor

@orlangur orlangur left a comment

Choose a reason for hiding this comment

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

  1. We must not change code of third-party libraries (like KnockoutJS)
  2. Changing fieldset name is backward-incompatible as to me: http://devdocs.magento.com/guides/v2.2/contributor-guide/backward-compatible-development/index.html#magento-apis

@orlangur
Copy link
Contributor

@davidangel, thanks for your contribution, it looks like all places which could be changed are already fixed in #11345.

@orlangur orlangur closed this Oct 20, 2017
@orlangur orlangur added this to the October 2017 milestone Oct 20, 2017
@davidangel
Copy link
Contributor Author

  1. Reverted. Is there a reason knockout.js isn't included via npm?
  2. I was not able to find any other instances of search_engine_optimisation in the codebase. I was able to find 7 other results for search_engine_optimization. Are we sure this change is not desired?

@orlangur
Copy link
Contributor

Is there a reason knockout.js isn't included via npm?

Yes. We don't want NodeJS on prod. It could be included via some Composer plugins but for now it works like that.

Are we sure this change is not desired?

Yeah, third-party customizations are relying on names. Keeping them unbroken is lesser evil than a typo in caption not shown to end user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants