)]}'
{"manifests/backend/rbd.pp":[{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"0839b728cd8610ff0e606d31aaf01786065c67d0","unresolved":false,"context_lines":[{"line_number":151,"context_line":"    }"},{"line_number":152,"context_line":"  }"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"  ensure_packages( \u0027ceph-common\u0027, {"},{"line_number":155,"context_line":"    ensure \u003d\u003e present,"},{"line_number":156,"context_line":"    name   \u003d\u003e $::cinder::params::ceph_common_package_name,"},{"line_number":157,"context_line":"    tag    \u003d\u003e \u0027cinder-support-package\u0027})"}],"source_content_type":"text/x-puppet","patch_set":3,"id":"7f515b1d_68f5c2ec","line":154,"updated":"2017-09-28 18:45:26.000000000","message":"nit: consider moving this block above L140 (just preceding it).\n\nThe reason is L140-152 is the set up for the code at L163. Adding this new block here kind of wedges it into the middle.\n\nSide note: is $override_match used anywhere?","commit_id":"a7c850f3c582a4abe6f75f224ad453bc2a102830"},{"author":{"_account_id":6796,"name":"Giulio Fidente","email":"gfidente@redhat.com","username":"gfidente"},"change_message_id":"f57af4b8dc83f2d90b039ee4b8a71578e3395791","unresolved":false,"context_lines":[{"line_number":151,"context_line":"    }"},{"line_number":152,"context_line":"  }"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"  ensure_packages( \u0027ceph-common\u0027, {"},{"line_number":155,"context_line":"    ensure \u003d\u003e present,"},{"line_number":156,"context_line":"    name   \u003d\u003e $::cinder::params::ceph_common_package_name,"},{"line_number":157,"context_line":"    tag    \u003d\u003e \u0027cinder-support-package\u0027})"}],"source_content_type":"text/x-puppet","patch_set":3,"id":"7f515b1d_a8685a1c","line":154,"in_reply_to":"7f515b1d_68f5c2ec","updated":"2017-09-28 19:16:38.000000000","message":"ack, I\u0027ll move it up","commit_id":"a7c850f3c582a4abe6f75f224ad453bc2a102830"}],"spec/defines/cinder_backend_rbd_spec.rb":[{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"d36dc8c4306966b4fb78cac48befefe96c36a25f","unresolved":false,"context_lines":[{"line_number":119,"context_line":"    end"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"    it \u0027should ensure that the cinder-volume sysconfig file is present\u0027 do"},{"line_number":122,"context_line":"      is_expected.to contain_package(\u0027ceph-common\u0027).with(:ensure \u003d\u003e :present)"},{"line_number":123,"context_line":"      is_expected.to contain_file(\u0027/etc/sysconfig/openstack-cinder-volume\u0027).with("},{"line_number":124,"context_line":"        :ensure \u003d\u003e \u0027present\u0027"},{"line_number":125,"context_line":"      )"}],"source_content_type":"text/x-ruby","patch_set":2,"id":"7f515b1d_6e75d563","line":122,"updated":"2017-09-28 12:59:07.000000000","message":"This seems redundant given L40, and not consistent with the \"it\" clause description. I\u0027m not sure if you are trying to test something RedHat specific (maybe the $ceph_common_package_name?).","commit_id":"5c5e6d425747493e9466f1a1248a1e61f94049bd"},{"author":{"_account_id":6796,"name":"Giulio Fidente","email":"gfidente@redhat.com","username":"gfidente"},"change_message_id":"5613d5d2ddda9a4723443c19f98723de18ffe34f","unresolved":false,"context_lines":[{"line_number":119,"context_line":"    end"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"    it \u0027should ensure that the cinder-volume sysconfig file is present\u0027 do"},{"line_number":122,"context_line":"      is_expected.to contain_package(\u0027ceph-common\u0027).with(:ensure \u003d\u003e :present)"},{"line_number":123,"context_line":"      is_expected.to contain_file(\u0027/etc/sysconfig/openstack-cinder-volume\u0027).with("},{"line_number":124,"context_line":"        :ensure \u003d\u003e \u0027present\u0027"},{"line_number":125,"context_line":"      )"}],"source_content_type":"text/x-ruby","patch_set":2,"id":"7f515b1d_5154fc52","line":122,"in_reply_to":"7f515b1d_6e75d563","updated":"2017-09-28 13:40:47.000000000","message":"yeah L114 is setting osfamily to RedHat vs L35 setting it to Debian so I it just makes sure we get it installed in both cases","commit_id":"5c5e6d425747493e9466f1a1248a1e61f94049bd"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"0839b728cd8610ff0e606d31aaf01786065c67d0","unresolved":false,"context_lines":[{"line_number":119,"context_line":"    end"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"    it \u0027should ensure that the cinder-volume sysconfig file is present\u0027 do"},{"line_number":122,"context_line":"      is_expected.to contain_package(\u0027ceph-common\u0027).with(:ensure \u003d\u003e :present)"},{"line_number":123,"context_line":"      is_expected.to contain_file(\u0027/etc/sysconfig/openstack-cinder-volume\u0027).with("},{"line_number":124,"context_line":"        :ensure \u003d\u003e \u0027present\u0027"},{"line_number":125,"context_line":"      )"}],"source_content_type":"text/x-ruby","patch_set":3,"id":"7f515b1d_e85d3215","line":122,"updated":"2017-09-28 18:45:26.000000000","message":"Picking up from our previous discussion on ps1.\n\nAlmost all of the tests happen to run with osfamily Debian (the stuff starting L38), and I think that\u0027s OK for ensuring the ceph-common package is present. The reason is the code that actually handles this (rbd.pp L154) isn\u0027t specific to an osfamily.\n\nThe other reason why I raised the point is I still feel this line is awkwardly wedged into the \"ensure the cinder-volume sysconfig file is present\" test.","commit_id":"a7c850f3c582a4abe6f75f224ad453bc2a102830"},{"author":{"_account_id":6796,"name":"Giulio Fidente","email":"gfidente@redhat.com","username":"gfidente"},"change_message_id":"f57af4b8dc83f2d90b039ee4b8a71578e3395791","unresolved":false,"context_lines":[{"line_number":119,"context_line":"    end"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"    it \u0027should ensure that the cinder-volume sysconfig file is present\u0027 do"},{"line_number":122,"context_line":"      is_expected.to contain_package(\u0027ceph-common\u0027).with(:ensure \u003d\u003e :present)"},{"line_number":123,"context_line":"      is_expected.to contain_file(\u0027/etc/sysconfig/openstack-cinder-volume\u0027).with("},{"line_number":124,"context_line":"        :ensure \u003d\u003e \u0027present\u0027"},{"line_number":125,"context_line":"      )"}],"source_content_type":"text/x-ruby","patch_set":3,"id":"7f515b1d_486e862f","line":122,"in_reply_to":"7f515b1d_e85d3215","updated":"2017-09-28 19:16:38.000000000","message":"so for RedHat the package name could be different, the code is the same in rbd.pp but the ceph_common_package_name might be different ... the test name should be updated if we keep this line here though\n\nI don\u0027t have a strong opinion about it though, I\u0027ll remove it given that as you pointed the pattern seems to be to have the majority of the tests running only once faking Debian","commit_id":"a7c850f3c582a4abe6f75f224ad453bc2a102830"}]}