Gilad Chaplik has posted comments on this change. Change subject: fixed generics in EnumTranslator, fixed typo ......................................................................
Patch Set 1: Code-Review+1 (1 comment) please change commit msg, Thanks. http://gerrit.ovirt.org/#/c/25605/1//COMMIT_MSG Commit Message: Line 7: fixed generics in EnumTranslator, fixed typo Line 8: Line 9: Parameterized EnumTranslator could accept any type, not just Enums, which is wrong. Constructor accepted also any type, but it was expected (not reinforced) that this type will be Class<? extends Enum<?>>. Line 10: --- Line 11: Notes/pending changes: public constructor should be probably removed, since it bypasses caching. Or, maybe better, the caching should be removed -- object creation should not pose a problem but I did not measure it. Method createAndTranslate should not be static -- there's not translate method. > IMO caching is not needed. And instantiating the class is not needed. Without going into details, I don't think we should write here needed future changes (Notes/pending changes as you call it)- There are more appropriate places (mailing list for example). The commit msg should be on the content of the fix, short and focused. Line 12: Not fixed error: not handled nullpointer exception in 'get' when null is passed to constructor. Line 13: Line 14: Change-Id: If835e719a497e7ab1db4fc4f9484354e838c60c5 -- To view, visit http://gerrit.ovirt.org/25605 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If835e719a497e7ab1db4fc4f9484354e838c60c5 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <[email protected]> Gerrit-Reviewer: Alona Kaplan <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Gilad Chaplik <[email protected]> Gerrit-Reviewer: Lior Vernia <[email protected]> Gerrit-Reviewer: Vojtech Szocs <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
