Alon Bar-Lev has posted comments on this change. Change subject: engine: remove the use of @LockIdNameAttribute ......................................................................
Patch Set 8: (2 comments) http://gerrit.ovirt.org/#/c/25944/8/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/LockProperties.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/LockProperties.java: Line 2: Line 3: public class LockProperties { Line 4: Line 5: public static enum Scope { Execution, Manual, None } Line 6: private Scope scope = Scope.None; > I don't see SCOPE_ACTION anywhere in this patch.. do you mean to add it to yes, the mission is to find these cases and add... Line 7: private boolean wait = true; Line 8: Line 9: private LockProperties() {} Line 10: Line 3: public class LockProperties { Line 4: Line 5: public static enum Scope { Execution, Manual, None } Line 6: private Scope scope = Scope.None; Line 7: private boolean wait = true; > That's what I'm concerned of, that the number of bugs that will be created for the first argument, I do not see that most cases wait is false, and I do think that most people will expect lock to wait unless something explicitly set. for the second argument, we have a lot of code in engine that current maintainers are used to, but is, and I want to use polite terms, not the best design. if we want to evolve, the legacy maintainers should cooperate, of course, but also endorse changes. there where quite some changes I was and am involved in, and after adjustment period are of a very positive impact. I opened the bug as result of a bug because of a severe inconsistent in use case of implementation (attribute + member), while we fix this we should also look into lock well known terms (scope, wait) and see how we can incorporate these into implementation, and not invent our own. so you are right, there will be adjustment period, but after that period we will be in better shape. Line 8: Line 9: private LockProperties() {} Line 10: Line 11: public boolean isWait() { -- To view, visit http://gerrit.ovirt.org/25944 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie57e4f7c00ebcd6a4e9e0e61b7d26f50f2d00858 Gerrit-PatchSet: 8 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ravi Nori <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Arik Hadas <[email protected]> Gerrit-Reviewer: Liron Ar <[email protected]> Gerrit-Reviewer: Omer Frenkel <[email protected]> Gerrit-Reviewer: Ravi Nori <[email protected]> Gerrit-Reviewer: Roy Golan <[email protected]> Gerrit-Reviewer: Sahina Bose <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[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
