Re: [vdsm] Patch review process
On Sun, Sep 09, 2012 at 02:33:00PM -0400, Alon Bar-Lev wrote: - Original Message - From: Adam Litke a...@us.ibm.com To: vdsm-devel@lists.fedorahosted.org Cc: Ryan Harper ry...@linux.vnet.ibm.com, Anthony Liguori aligu...@linux.vnet.ibm.com Sent: Sunday, September 9, 2012 8:27:30 PM Subject: [vdsm] Patch review process While discussing gerrit recently, I learned that some people use gerrit simply to host work-in-progress patches and they don't intend for those to be reviewed. How can a reviewer recognize this and skip those patches when choosing what to review? Is there a way to mark certain patches as more important and others as drafts? Yes. See [1]. $ git push upstream HEAD:refs/drafts/master/description Thanks for pointing it out. It would be nice if we could get people to start pushing WIP patches to drafts now that we have this feature. [1] http://gerrit-documentation.googlecode.com/svn/ReleaseNotes/ReleaseNotes-2.3.html -- Adam Litke a...@us.ibm.com IBM Linux Technology Center ___ vdsm-devel mailing list vdsm-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel
Re: [vdsm] Change in vdsm[master]: engine.py: fail if Password doesn't match
* dougsl...@redhat.com dougsl...@redhat.com [2012-09-11 01:38]: Douglas Schilling Landgraf has uploaded a new change for review. Change subject: engine.py: fail if Password doesn't match .. engine.py: fail if Password doesn't match Currently, if users in the Node TUI add the password to include the node through Engine and passwords doesn't match, no failure message will show and it will continue. This patch will show to users a failure message if the passwords doesn't match. Bug-Id: https://bugzilla.redhat.com/show_bug.cgi?id=854457 Test: * Install Node * Select oVirt Engine Tab * Add passwords that doesn't match and click 'Apply' Change-Id: I143906eb6ce61037418eac25567496c6628aede9 Signed-off-by: Douglas Schilling Landgraf dougsl...@redhat.com --- M vdsm_reg/engine.py.in 1 file changed, 11 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/17/7917/1 diff --git a/vdsm_reg/engine.py.in b/vdsm_reg/engine.py.in index 74fe07a..b104635 100644 --- a/vdsm_reg/engine.py.in +++ b/vdsm_reg/engine.py.in @@ -42,6 +42,8 @@ VDSM_REG_CONFIG = /etc/vdsm-reg/vdsm-reg.conf VDC_HOST_PORT = 443 TIMEOUT_FIND_HOST_SEC = 5 +PASSWORD_MATCH = 0 +PASSWORD_DOESNT_MATCH = 1 fWriteConfig = 0 def set_defaults(): @@ -186,6 +188,7 @@ self.root_password_2.setCallback(self.password_check_callback) pw_elements.setField(self.root_password_2, 1, 2) self.pw_msg = Textbox(60, 6, , wrap=1) +self.pw_resp = PASSWORD_MATCH elements.setField(pw_elements, 0, 6, anchorLeft=1) elements.setField(self.pw_msg, 0, 7, padding = (0,0,0,0)) @@ -212,9 +215,17 @@ def password_check_callback(self): resp, msg = password_check(self.root_password_1.value(), self.root_password_2.value()) self.pw_msg.setText(msg) +self.pw_resp = resp return def action(self): +# Show error if the password informed by user doesn't match +if self.pw_resp == PASSWORD_DOESNT_MATCH and len(self.root_password_1.value()) 0 or \ +self.pw_resp == PASSWORD_DOESNT_MATCH and len(self.root_password_2.value()) 0: +msg = Password Do Not Match! +ButtonChoiceWindow(self.ncs.screen, @ENGINENAME@, msg, buttons = ['Ok']) +return + What about moving the Button popup to an else clause of the if-check that's already validating the password ? if self.root_password_1.value() != and self.root_password_2.value() != and self.root_password_1.value() == self.root_password_2.value(): # set password ... else: ButtonChoiceWindow() return? # To manage the engine_server_port value, use enginePort var to avoid # TUI putting the port value in the screen when you are just changing # the value and not asking to draw/print it. -- To view, visit http://gerrit.ovirt.org/7917 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I143906eb6ce61037418eac25567496c6628aede9 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Douglas Schilling Landgraf dougsl...@redhat.com ___ vdsm-patches mailing list vdsm-patc...@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches -- Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx ry...@us.ibm.com ___ vdsm-devel mailing list vdsm-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel
Re: [vdsm] Change in vdsm[master]: engine.py: fail if Password doesn't match
* Douglas Landgraf dougsl...@redhat.com [2012-09-11 12:15]: Hi Ryan, On 09/11/2012 10:21 AM, Ryan Harper wrote: * dougsl...@redhat.comdougsl...@redhat.com [2012-09-11 01:38]: Douglas Schilling Landgraf has uploaded a new change for review. Change subject: engine.py: fail if Password doesn't match .. engine.py: fail if Password doesn't match Currently, if users in the Node TUI add the password to include the node through Engine and passwords doesn't match, no failure message will show and it will continue. This patch will show to users a failure message if the passwords doesn't match. Bug-Id: https://bugzilla.redhat.com/show_bug.cgi?id=854457 Test: * Install Node * Select oVirt Engine Tab * Add passwords that doesn't match and click 'Apply' Change-Id: I143906eb6ce61037418eac25567496c6628aede9 Signed-off-by: Douglas Schilling Landgrafdougsl...@redhat.com --- M vdsm_reg/engine.py.in 1 file changed, 11 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/17/7917/1 diff --git a/vdsm_reg/engine.py.in b/vdsm_reg/engine.py.in index 74fe07a..b104635 100644 --- a/vdsm_reg/engine.py.in +++ b/vdsm_reg/engine.py.in @@ -42,6 +42,8 @@ VDSM_REG_CONFIG = /etc/vdsm-reg/vdsm-reg.conf VDC_HOST_PORT = 443 TIMEOUT_FIND_HOST_SEC = 5 +PASSWORD_MATCH = 0 +PASSWORD_DOESNT_MATCH = 1 fWriteConfig = 0 def set_defaults(): @@ -186,6 +188,7 @@ self.root_password_2.setCallback(self.password_check_callback) pw_elements.setField(self.root_password_2, 1, 2) self.pw_msg = Textbox(60, 6, , wrap=1) +self.pw_resp = PASSWORD_MATCH elements.setField(pw_elements, 0, 6, anchorLeft=1) elements.setField(self.pw_msg, 0, 7, padding = (0,0,0,0)) @@ -212,9 +215,17 @@ def password_check_callback(self): resp, msg = password_check(self.root_password_1.value(), self.root_password_2.value()) self.pw_msg.setText(msg) +self.pw_resp = resp return def action(self): +# Show error if the password informed by user doesn't match +if self.pw_resp == PASSWORD_DOESNT_MATCH and len(self.root_password_1.value()) 0 or \ +self.pw_resp == PASSWORD_DOESNT_MATCH and len(self.root_password_2.value()) 0: +msg = Password Do Not Match! +ButtonChoiceWindow(self.ncs.screen, @ENGINENAME@, msg, buttons = ['Ok']) +return + What about moving the Button popup to an else clause of the if-check that's already validating the password ? if self.root_password_1.value() != and self.root_password_2.value() != and self.root_password_1.value() == self.root_password_2.value(): # set password ... else: ButtonChoiceWindow() return? Adding directly the else statement will break the flow, since it requires self.root_password_1.value() and self.root_password_2.value() be different of . If user don't want to use such feature, both fields will contain the value and the flow should continue . Right, if they're not setting a password at all... I suppose an elif instead with a check for non-empty password len in either pw1 or 2 would fix that. I am going to send a new patch to avoid a second if statement. OK Looks like you have decided to review the code replying to mailing list instead of to use the gerrit flow. Can you please follow the gerrit flow as well since currently most of developers are using it? At least, until the project change it. Sure, I'll try to keep current on gerrit as well. Thanks for the review. Sure. -- Cheers Douglas -- Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx ry...@us.ibm.com ___ vdsm-devel mailing list vdsm-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel