Shireesh Anjal has posted comments on this change. Change subject: gluster: Make set option a step of create volume ......................................................................
Patch Set 5: Have changed getJobMessageProperties() as per Moti's suggestion. However I have a couple of concerns with the design of this method. 1) It is a method which is supposed to initialize a class level variable, and return it. I think that is not good. It should either - be a "void" method that is used to initialize the variable, or - return a newly created map that is later assigned to the class level variable. 2) Since it is supposed to return a map of name-value pairs that can be used as dynamic values in the description of the job, it should be executed only once for a given instance of a command. And hence, developer should not be expected to optimize it for a scenario where getJobMessageProperties() is called multiple times on the same command. -- To view, visit http://gerrit.ovirt.org/10905 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie0f22e9862584f1a616fcc01e8a80b7d5a5ffc78 Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Shireesh Anjal <san...@redhat.com> Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Shireesh Anjal <san...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
