Juan Hernandez has posted comments on this change. Change subject: packaging: added updates to pgpass creation and reading ......................................................................
Patch Set 5: (4 inline comments) Looks good to me, some minor comments inside. .................................................... File packaging/fedora/setup/basedefs.py Line 7: DB_ADMIN="postgres" Isn't this par of another change, I mean the "MY_NAME" variable. .................................................... File packaging/fedora/setup/common_utils.py Line 597: if (os.path.exists(basedefs.DB_PASS_FILE)): No need for the outer parenthesis here ^ . Line 607: if inDbAdminSection and 'admin' in param and \ This 'admin' in param comparision is a bit strange. Shouldn't it be param == 'admin"? I mean, this will give True for params like 'badadmin'. I know this is not going to happen, but it puzzled me a bit, so it will probably puzzle the next person to read this code as well. .................................................... File packaging/fedora/setup/engine-setup.py Line 1130: # ##### End of oVirt Engine DB settings section." Nice comment, I would suggest moving it to the docstring at the beginning of the function. -- To view, visit http://gerrit.ovirt.org/5627 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5619f3b47959efa2f24e2601cd0a8793150828b Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alex Lourie <[email protected]> Gerrit-Reviewer: Alex Lourie <[email protected]> Gerrit-Reviewer: Juan Hernandez <[email protected]> Gerrit-Reviewer: Moran Goldboim <[email protected]> Gerrit-Reviewer: Ofer Schreiber <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
