Ayal Baron has posted comments on this change.

Change subject: shareVolumeRollback() unique call likes to raise on the 
unexpected.
......................................................................


Patch Set 2: I would prefer that you didn't submit this

(2 inline comments)

....................................................
Commit Message
Line 3: AuthorDate: 2013-01-27 11:02:13 +0200
Line 4: Commit:     Eduardo Warszawski <[email protected]>
Line 5: CommitDate: 2013-01-28 10:17:37 +0200
Line 6: 
Line 7: shareVolumeRollback() unique call likes to raise on the unexpected.
I have no idea what this sentence means
Line 8: 
Line 9: Looking in to Volume.share() (unique) call to shareVolumeRollback()
Line 10: seems evident that it should raise if the recovery failed.
Line 11: The try blocks removed silently blow cases when the recovery path


Line 9: Looking in to Volume.share() (unique) call to shareVolumeRollback()
Line 10: seems evident that it should raise if the recovery failed.
Line 11: The try blocks removed silently blow cases when the recovery path
Line 12: failed to remove the link (if it exists) because any other reason
Line 13: that an inexistent link; this is clearly unintentional.
I had no idea what you were trying to say here until I read the code (which 
beats the purpose of the commit message).
The reference to 'unique' still remains in the dark (called only here? why is 
that relevant in the commit message?).

s/inexistent/nonexistent/

Since the links are being removed with safeUnlink then your example is clearly 
wrong (as it would not raise an error).
Line 14: 
Line 15: Change-Id: I8f01ff2d30ffecc05b9cfd4978f79003a8032270


--
To view, visit http://gerrit.ovirt.org/11422
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8f01ff2d30ffecc05b9cfd4978f79003a8032270
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Eduardo <[email protected]>
Gerrit-Reviewer: Yeela Kaplan <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to