Martin Mucha has posted comments on this change. Change subject: webadmin: removed caching from EnumTranslator. ......................................................................
Patch Set 3: Ok, I reverted all unrelated changes introduced in this path. Question 1: What's the definition of "related changes"? If I have to step into badly formatted method or overgrown method or what ever, can I fix it or should I do it as a separate commit on which my commit depends? If I separate it into smaller commit (which I would like to do) I'll be dependant on those commit approval which can take long time. And neglecting to fix evident errors just beacause don't want to be related to those fixes or aren't relevant to mine changes stop code improvements. So what's the corrent way out of this 3? Same commit / another commit / do not commit&deal with it. Question 2: In EnumTranslator I wrote a memo. Is it visible to others? I mean it, I'm not trying to be sarcastic :) If not, I wrote there, that 'get' method was duplicate and once returned 'constants.notAvailableLabel();' and once 'null'. I'm not sure that there is a correct way how to fix it in a bullet-proof way. One can depend on null others on 'notAvailableLabel'; I tried to check that out, but that's impossible. I've reverted it back to state, where 'notAvailableLabel is returned' -- which breaks all code, which expects 'null'. And a last request for change. I think that following code is just fine: Translator translator = EnumTranslator.getInstance(); stnm.setDescription(translator.containsKey(eventType) ? translator.get(eventType) More efficient, robust and more readable than proposed stnm.setDescription(EnumTranslator.getInstance().containsKey(eventType) ? EnumTranslator.getInstance().get(eventType) thus I did not changed that. You're really sure you want it *that* way? -- To view, visit http://gerrit.ovirt.org/26048 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3df424d8e7a35ed249b2760da79deba7db31b785 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <[email protected]> Gerrit-Reviewer: Alona Kaplan <[email protected]> Gerrit-Reviewer: Lior Vernia <[email protected]> Gerrit-Reviewer: Martin Mucha <[email protected]> Gerrit-Reviewer: Vojtech Szocs <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
