Alona Kaplan has posted comments on this change.
Change subject: webadmin: user portal and webadmin localization
......................................................................
Patch Set 7:
Hi Vojtech, thanks for the comments.
* Why was UICommonWeb.gwt.xml deleted?
-I have deleted it by mistake. I have already fixed it,
I will post it and some more little fixes later today.
* In Constants/Messages interfaces we're already using @DefaultStringValue. Do
we really need to have default Constant/Messages properties files as well?
- I talked about it with Einav on Thursday. I will delete the default
properties files. She wanted me to add them in order to have properties files
example to send to the translators. But I will write a script to generate them
automaticly and they won't be part of the build.
* Besides using Constants/Messages interfaces throughout Frontend and UiCommon,
are there any other major infrastructure changes?
- No.
* In Enums.properties, there is some ">>>>>>> webadmin" stuff, I guess because
of some rebase?
- Thanks, I will fix it.
* In ConstantsManager, I'd add a private constructor and make ConstantsManager
final to indicate it shouldn't be used directly, but via getInstance() (just a
suggestion).
- I will fix it to be a correct singleton:)
* Currently, we have 2 ways to localize View/UiBinder component: 1) have
*.ui.xml declare <ui:with...> for constants, or 2) have localize() method in
View.
- Some comment about this issue
- The future checkstyle (next patch) and the eclipse checkstyle can find non
externalized string (and mark them as an error until you add NON-NLS comment or
externalize them) just in *.java files. So way 2 is preferred!
- I didn't want to change the views code, so if the non externalized string was
in the ui.xml I left it there and used way 1.
- In way 2- we don't really localize everything in localize method. There is a
mess there. There is no specific place to localize. I think we have to refactor
localize method name to initialize or something like that. What do you think?
* I guess NLS comment stuff ("//$NON-NLS-N$") doesn't apply to strings in
annotations, e.g. @DefaultStringValue("..."), is this right?
- Yes, you are right.
--
To view, visit http://gerrit.ovirt.org/3612
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74c56a3aed35dec0a378efc58a2e44569eff6089
Gerrit-PatchSet: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alona Kaplan <[email protected]>
Gerrit-Reviewer: Alona Kaplan <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
