Alon Bar-Lev has posted comments on this change. Change subject: aaa: Remove dependency at builtin on Common config ......................................................................
Patch Set 10: (3 comments) looks good! minor comments. http://gerrit.ovirt.org/#/c/27607/10/backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/kerberosldap/Utils.java File backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/kerberosldap/Utils.java: Line 10: import org.ovirt.engine.api.extensions.Base; Line 11: Line 12: public class Utils { Line 13: Line 14: public static void setDefaults(Properties conf) { if this utils... I would have passed the calling class as parameter to get this defaults... so that you can have multiple... or better just pass InputStream... Line 15: { Line 16: Properties defaults = new Properties(); Line 17: try { Line 18: defaults.load(Utils.class.getResourceAsStream("defaults.properties")); Line 14: public static void setDefaults(Properties conf) { Line 15: { Line 16: Properties defaults = new Properties(); Line 17: try { Line 18: defaults.load(Utils.class.getResourceAsStream("defaults.properties")); you should always read properties with reader to allow utf... and even if not, you should close the stream... Line 19: for (Map.Entry<Object, Object> entry : defaults.entrySet()) { Line 20: putIfAbsent(conf, (String) entry.getKey(), (String) entry.getValue()); Line 21: } Line 22: http://gerrit.ovirt.org/#/c/27607/10/packaging/services/ovirt-engine/ovirt-engine.conf.in File packaging/services/ovirt-engine/ovirt-engine.conf.in: Line 100: # Line 101: # -Dmy.param=my.value -Dmy.flag -Dyour.param=your.value Line 102: # Line 103: ENGINE_PROPERTIES="" Line 104: ENGINE_PROPERTIES="${ENGINE_PROPERTIES} jsse.enableSNIExtension=false java.security.krb5.conf=\"${ENGINE_ETC}/krb5.conf\"" it should be at new line to allow readability and quote the entire string: ENGINE_PROPERTIES="${ENGINE_PROPERTIES} \"java.security.krb5.conf=${ENGINE_ETC}/krb5.conf\"" Line 105: Line 106: # Line 107: # Extra Java arguments to be added to command-line. Line 108: # -- To view, visit http://gerrit.ovirt.org/27607 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1384a99f73ab605b61bce8dcdfd63e222b0001fa Gerrit-PatchSet: 10 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
