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

fix message lookup for core nodes in case of i18 locales directory ex… #1616

Merged
merged 1 commit into from Feb 23, 2018

Conversation

HiroyasuNishiyama
Copy link
Member

@HiroyasuNishiyama HiroyasuNishiyama commented Feb 7, 2018

…ists

Before you hit that Submit button....

Please read our contribution guidelines
before submitting a pull-request.

Types of changes

What types of changes does your code introduce?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

If you want to raise a pull-request with a new feature, or a refactoring
of existing code, it may well get rejected if it hasn't been discussed on
the mailing list or
slack team first.

Proposed changes

Describe the nature of this change. What problem does it address?

As reported by issue #1615, message catalogue lookup for core nodes become failed when i18n locales directory (nodes/core/<category>/locales) exists.

This is caused by loadNodeConfig function in red/runtime/nodes/registry/loader.js.
If a locales directory exists, loadNodeConfig try to look up message catalogue (.json) for nodes in modules under the directory. Because the combined message catalogue for core nodes are placed at different location, this look up fails and message catalogue is corrupted.

This PR fixes this problem by skipping message catalogue look up for core nodes in loadNodeConfig.

My proposal is to place info text of core nodes under nodes/core/locales (or separate message catalogue of core nodes from messages.json). But, this may needs more discussion. So, this PR expects to place info text under nodes/core/<category>/locales.

Checklist

Put an x in the boxes that apply

  • I have read the contribution guidelines
  • For non-bugfix PRs, I have discussed this change on the mailing list/slack team.
  • I have run grunt to verify the unit tests pass
  • I have added suitable unit tests to cover the new/changed functionality

@coveralls
Copy link

coveralls commented Feb 7, 2018

Coverage Status

Coverage remained the same at 87.755% when pulling ce9643d on node-red-hitachi:fix-i18n-message-lookup into b378670 on node-red:master.

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.

None yet

3 participants