Hi Nice initiative! I reviewed some suspect infra-related flows, and added comments. I must say that it is not very clear what you're looking for. In one of the cases I reviewed, for example, there was a comment explaining the logic. And, not all cases require a helper method, as some should be determined only in another helper class (like the case I reviewed, which eventually is SLA-related). It seems like you want people to do is: 1. Go over conditions that are relevant to you, and make sure they are complete with regards to the possible VM statuses (do you really care about a comment explaining why is the logic that way?)

2. Write down a name of a method that might help your code look better, if any (if there isn't a clear one, like in the case above, then I guess it should either move to another helper that is related to the domain in question, or left as is with comparison). Am I correct? Also, I'd want to see these helper methods in the VM class rather than in some helper class. myVM.isRunning() is more readable than than VMStatusHelper.isRunning(myVm), but that's just my opinion.

Thanks! Oved ----- Original Message ----- > From: "Shmuel Melamud" <smelamud(a)redhat.com&gt; > To: devel(a)ovirt.org > Sent: Tuesday, July 14, 2015 12:12:52 PM > Subject: [ovirt-devel] VM status checks - request for comments > > Hi! > > I'm currently working on creation of new VMStatus helper functions that may > be used to check VM status when needed being all-purpose, robust and > universal. > > For this purpose I've created a table of all places where VM status is > checked, using current VMStatus functions, long lines of comparisons or some > indirect way like validators. (Sure, I didn't include some cases. VmAnalyzer > logic doesn't fit here. Mappings, rendering, message selection, changing > states in migration process - it is not the case when helper functions may > help.) > > Here is the table: > https://docs.google.com/a/redhat.com/spreadsheets/d/1yb-JdTAGh_4bOC6tL7KL... > > Many lines are raising questions - why these states are allowed and these are > not? What to do in Unknown state, in migration, hibernation, > WaitingForLaunch? > > That's why I'm asking those of you who're familiar with some places to review > them and write down your comments. What logic is behind the selection of the > states? What states should be here, or shouldn't be here? This will give > invaluable help to create helper functions that are really helpful ;) > > -- > Shmuel > _______________________________________________ > Devel mailing list > Devel(a)ovirt.org > http://lists.ovirt.org/mailman/listinfo/devel > > > _______________________________________________ Devel mailing list Devel(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/devel