You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by ns7381 <gi...@git.apache.org> on 2018/01/17 06:31:01 UTC
[GitHub] nifi pull request #2408: localization using the JSTL standard fmt tag for mu...
GitHub user ns7381 opened a pull request:
https://github.com/apache/nifi/pull/2408
localization using the JSTL standard fmt tag for multilingualization and nf._.msg() function in resource.js
jira issue: https://issues.apache.org/jira/browse/NIFI-4785
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ns7381/nifi NIFI-4785
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi/pull/2408.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2408
----
commit 1bdce66e0e468846953d21b37e8bfa16c48d3d73
Author: ningsheng <ns...@...>
Date: 2018-01-17T06:25:12Z
localization using the JSTL standard fmt tag for multilingualization and nf._.msg() function in resource.js
----
---
[GitHub] nifi issue #2408: localization using the JSTL standard fmt tag for multiling...
Posted by ns7381 <gi...@git.apache.org>.
Github user ns7381 commented on the issue:
https://github.com/apache/nifi/pull/2408
@mcgilman OK. If have any problem, please timely communicate
---
[GitHub] nifi issue #2408: localization using the JSTL standard fmt tag for multiling...
Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on the issue:
https://github.com/apache/nifi/pull/2408
@ns7381 I've started reviewing the PR. This is a great addition. Thank you for taking the time to post it. Before we merge it in, I have a couple requests.
- The changes appear to cause the icons in the toolbar at the top to become disable with a vanilla unsecured instance. I haven't been able to continue verifying functionality as I'm blocked on this issue.
- The client-side code is structured in a modular format. Each module has any dependent modules passed into it. Rather than directly referencing nf._ on the global scope throughout the code base, can you please pass in a reference to nf._ in each module that references it? See the top of any nf-<module-name>.js for an example.
- The Messages-<county-code>.properties are under src/main/java. Can these be moved to src/main/resources?
- Some of the keys in resources.js have generic names like Message11. Can we use meaningful names throughout?
- I think we may need to introduce some defensive programming in some cases now that the values which were previously string literals have been abstracted and accessed via a lookup. Since there is no intention to support markup in the values, we should not pass them into functions that support interpreting markup. Instead that should only be set via `.text(...)` or escaped prior to passing in to `.html(...)`, `.append(...)`, etc. You should see other instances of this throughout the codebase, but I am happy to help identify specific cases as the review progresses.
- What strategies are available for applying localization to messages returned from the server? Should a follow-on JIRA be filed to help in this area?
Thanks!
---
[GitHub] nifi issue #2408: localization using the JSTL standard fmt tag for multiling...
Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on the issue:
https://github.com/apache/nifi/pull/2408
@ns7381 Thanks for the PR! I plan to start reviewing this soon.
---
[GitHub] nifi issue #2408: localization using the JSTL standard fmt tag for multiling...
Posted by ns7381 <gi...@git.apache.org>.
Github user ns7381 commented on the issue:
https://github.com/apache/nifi/pull/2408
@mcgilman OK. If have any problem, please timely communicate
---