Update patch set 2

Patch Set 2: Code-Review-1

(6 comments)

Patch-set: 2
Label: Code-Review=-1

086785d96aead148fffb7ecd6af5778a10097de6

@@ -1,5 +1,22 @@
 {
   "comments": [
+    {
+      "key": {
+        "uuid": "7a5de9d1_687f3be0",
+        "filename": "defaults/main.yml",
+        "patchSetId": 2
+      },
+      "lineNbr": 33,
+      "author": {
+        "id": 6816
+      },
+      "writtenOn": "2016-02-01T18:22:46Z",
+      "side": 1,
+      "message": "This seems to me to make the use of overrides less user friendly. If the package list var name is consistent between platforms then we don\u0027t have to know the platform/installer in order to know which var to override.\n\nThis is why the bootstrap-host role implements a vars file per platform:\n\nhttps://github.com/openstack/openstack-ansible/blob/master/tests/roles/bootstrap-host/vars/ubuntu.yml\n\nhttps://github.com/openstack/openstack-ansible/blob/master/tests/roles/bootstrap-host/tasks/main.yml#L21-L30",
+      "revId": "086785d96aead148fffb7ecd6af5778a10097de6",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
+    },
     {
       "key": {
         "uuid": "7a5de9d1_87662b28",
@@ -35,6 +52,75 @@
       "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
       "unresolved": false
     },
+    {
+      "key": {
+        "uuid": "7a5de9d1_48bd9fd6",
+        "filename": "tasks/rsyslog_server_install.yml",
+        "patchSetId": 2
+      },
+      "lineNbr": 19,
+      "author": {
+        "id": 6816
+      },
+      "writtenOn": "2016-02-01T18:22:46Z",
+      "side": 1,
+      "message": "I do like the idea of doing a pre-flight check and failing fast, but the way that\u0027s done here is quite laborious. I\u0027d rather see this either directly in the main.yml or as an included task file first in the main.yml as is done in the tests/bootstrap-host role. That way the role fails fast if the appropriate platform is not catered for. It could potentially be added to the pre_install task list too, but it seems like other things happen there.",
+      "parentUuid": "7a5de9d1_02eb09e0",
+      "revId": "086785d96aead148fffb7ecd6af5778a10097de6",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
+    },
+    {
+      "key": {
+        "uuid": "7a5de9d1_88acf78c",
+        "filename": "tasks/rsyslog_server_install.yml",
+        "patchSetId": 2
+      },
+      "lineNbr": 30,
+      "author": {
+        "id": 6816
+      },
+      "writtenOn": "2016-02-01T18:22:46Z",
+      "side": 1,
+      "message": "This seems to be quite a laborious way of doing it. Why not have a special task file that does any apt related things, similar to https://github.com/openstack/openstack-ansible/blob/master/tests/roles/bootstrap-host/tasks/main.yml#L41-L46 ?",
+      "revId": "086785d96aead148fffb7ecd6af5778a10097de6",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
+    },
+    {
+      "key": {
+        "uuid": "7a5de9d1_88571725",
+        "filename": "tasks/rsyslog_server_install.yml",
+        "patchSetId": 2
+      },
+      "lineNbr": 69,
+      "author": {
+        "id": 6816
+      },
+      "writtenOn": "2016-02-01T18:22:46Z",
+      "side": 1,
+      "message": "Any reason for not using the yum module for this? http://docs.ansible.com/ansible/yum_module.html",
+      "revId": "086785d96aead148fffb7ecd6af5778a10097de6",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
+    },
+    {
+      "key": {
+        "uuid": "7a5de9d1_88a95726",
+        "filename": "tasks/rsyslog_server_install.yml",
+        "patchSetId": 2
+      },
+      "lineNbr": 70,
+      "author": {
+        "id": 6816
+      },
+      "writtenOn": "2016-02-01T18:22:46Z",
+      "side": 1,
+      "message": "This template appears to be missing?",
+      "revId": "086785d96aead148fffb7ecd6af5778a10097de6",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
+    },
     {
       "key": {
         "uuid": "7a5de9d1_4734b348",
@@ -51,6 +137,24 @@
       "revId": "086785d96aead148fffb7ecd6af5778a10097de6",
       "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
       "unresolved": false
+    },
+    {
+      "key": {
+        "uuid": "7a5de9d1_08e7a7ac",
+        "filename": "tasks/rsyslog_server_pre_install.yml",
+        "patchSetId": 2
+      },
+      "lineNbr": 26,
+      "author": {
+        "id": 6816
+      },
+      "writtenOn": "2016-02-01T18:22:46Z",
+      "side": 1,
+      "message": "This is why I think the distro check must happen before anything that changes a target. The role should fail fast if the distro is not supported.",
+      "parentUuid": "7a5de9d1_4734b348",
+      "revId": "086785d96aead148fffb7ecd6af5778a10097de6",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
     }
   ]
 }