)]}'
{"lib/puppet/provider/keystone.rb":[{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"032d6f7d82db72d0f05b2690565365ec93a2bb32","unresolved":false,"context_lines":[{"line_number":72,"context_line":"  end"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"  def self.default_domain"},{"line_number":75,"context_line":"    \u0027Default\u0027"},{"line_number":76,"context_line":"  end"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"  def self.domain_name_from_id(id)"}],"source_content_type":"text/x-ruby","patch_set":23,"id":"5a2ca52d_4c049114","line":75,"updated":"2015-10-13 14:58:22.000000000","message":"@Richard: for me it was a quick way to unplug the calculation of the default domain whithout breaking everything.  Actually while greping the code for it, I notice that you can set the default domain in the init.pp file.  I think that would be unexpected to have this functionality there and then blindly use Default everywhere.  That doesn\u0027t make sense.  Now that the patch work, I\u0027m going to check if I can reintroduce some calculation in Default: \"if the user specified an alternative default domain in the catalog then use it\".  I won\u0027t go as far as it used to be (check the existing resource in the system) as 1/ I don\u0027t have access to this when catalog is computed 2/ the ini file may be on another system.  But catalog searching is robust.\n@Gilles: it is already in keystone.rb.  Did I miss something ?","commit_id":"148b7edcc73f36ed0572a083c33573715074e52b"},{"author":{"_account_id":9983,"name":"Richard Megginson","email":"rmeggins@redhat.com","username":"rmeggins"},"change_message_id":"d72bef06a1dab014a0b9fd47779009632e8f80eb","unresolved":false,"context_lines":[{"line_number":72,"context_line":"  end"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"  def self.default_domain"},{"line_number":75,"context_line":"    \u0027Default\u0027"},{"line_number":76,"context_line":"  end"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"  def self.domain_name_from_id(id)"}],"source_content_type":"text/x-ruby","patch_set":23,"id":"7a2fa921_6ea933df","line":75,"updated":"2015-10-12 18:56:34.000000000","message":"This can cause confusion because method \u0027default_domain_id\u0027 refers to the \u0027identity/default_domain_id\u0027 setting in keystone.conf, but the method \u0027default_domain\u0027 means \"Default\".  I think we should either just use \u0027Default\u0027 instead of the method \u0027default_domain\u0027, or rename the method so as not to cause confusion with \u0027default_domain_id\u0027.","commit_id":"148b7edcc73f36ed0572a083c33573715074e52b"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"300a2e639b94c036c274be5db9058935863445c9","unresolved":false,"context_lines":[{"line_number":72,"context_line":"  end"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"  def self.default_domain"},{"line_number":75,"context_line":"    \u0027Default\u0027"},{"line_number":76,"context_line":"  end"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"  def self.domain_name_from_id(id)"}],"source_content_type":"text/x-ruby","patch_set":23,"id":"7a2fa921_0b76633e","line":75,"in_reply_to":"7a2fa921_6ea933df","updated":"2015-10-13 01:49:06.000000000","message":"Since several providers will make use of such methods I believe a better place would be lib/puppet/provider/keystone.rb. \nThis way there won\u0027t be any confusion. I mean if that method is needed an not generalized yet.","commit_id":"148b7edcc73f36ed0572a083c33573715074e52b"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"c375e58da3c724bc36e9f822c4a24ddccd7f8cc7","unresolved":false,"context_lines":[{"line_number":77,"context_line":"    @@default_domain || \u0027Default\u0027"},{"line_number":78,"context_line":"  end"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"  def self.default_domain\u003d(value)"},{"line_number":81,"context_line":"    @@default_domain \u003d value"},{"line_number":82,"context_line":"  end"},{"line_number":83,"context_line":""}],"source_content_type":"text/x-ruby","patch_set":27,"id":"5a2ca52d_580aaf22","line":80,"updated":"2015-10-15 11:39:17.000000000","message":"Not really, as the catalog is searched for the keystone_domain with default parameter set to true.  Here the @@default_domain is more a caching mecanism to prevent the catalog to be searched several time than a unique truth point.  But, thinking about it, make me think that it would be better as my catalog search is fragile.  Currently, if the user create two keystone_domain resources whose one of them is ensure \u003d\u003e present and the other ensure absent, my code will randomly returns one of them.\n\nI\u0027m thinking about what is the best way to fix this, and maybe setting this is keystone_domain makes more sense.\n\nThanks.","commit_id":"e8eb8710e2c0075ddc5eb300e1b03472de1eb337"},{"author":{"_account_id":9983,"name":"Richard Megginson","email":"rmeggins@redhat.com","username":"rmeggins"},"change_message_id":"ae21102dc21a5d1783517801567c7f85c1ebea18","unresolved":false,"context_lines":[{"line_number":77,"context_line":"    @@default_domain || \u0027Default\u0027"},{"line_number":78,"context_line":"  end"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"  def self.default_domain\u003d(value)"},{"line_number":81,"context_line":"    @@default_domain \u003d value"},{"line_number":82,"context_line":"  end"},{"line_number":83,"context_line":""}],"source_content_type":"text/x-ruby","patch_set":27,"id":"5a2ca52d_fe885a24","line":80,"updated":"2015-10-15 01:04:31.000000000","message":"do you need to call `default_domain\u003d` in `keystone_domain/openstack.rb` where you call `default_domain_id\u003d` so that the default domain name and id are in sync?","commit_id":"e8eb8710e2c0075ddc5eb300e1b03472de1eb337"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"5727c631633ab3af8136fdd82d279f5388ecacea","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    end"},{"line_number":44,"context_line":"  end"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"  def self.default_domain_id\u003d(id)"},{"line_number":47,"context_line":"    @@default_domain_id \u003d id"},{"line_number":48,"context_line":"  end"},{"line_number":49,"context_line":""}],"source_content_type":"text/x-ruby","patch_set":42,"id":"1a26ad4f_934ee194","line":46,"updated":"2015-10-30 17:08:32.000000000","message":"I\u0027ve been shy with this one.  It is used in keystone_domain to set the class instance variable @@default_domain_id and then is the keystone_domain provider.  I could try to get rid of it altogher and see what happen.  But maybe another review later on ?","commit_id":"c1e75919063f23991757f93255d1ea80b3587007"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"02ce99c70e427040f3ce001c637a24c71d3e875a","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    end"},{"line_number":44,"context_line":"  end"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"  def self.default_domain_id\u003d(id)"},{"line_number":47,"context_line":"    @@default_domain_id \u003d id"},{"line_number":48,"context_line":"  end"},{"line_number":49,"context_line":""}],"source_content_type":"text/x-ruby","patch_set":42,"id":"1a26ad4f_c2d10059","line":46,"updated":"2015-10-30 02:29:27.000000000","message":"Is this used anywhere?","commit_id":"c1e75919063f23991757f93255d1ea80b3587007"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"8710ae0802ff82b285267bae37a12e55dda885aa","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    end"},{"line_number":44,"context_line":"  end"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"  def self.default_domain_id\u003d(id)"},{"line_number":47,"context_line":"    @@default_domain_id \u003d id"},{"line_number":48,"context_line":"  end"},{"line_number":49,"context_line":""}],"source_content_type":"text/x-ruby","patch_set":42,"id":"1a26ad4f_960c7dee","line":46,"in_reply_to":"1a26ad4f_934ee194","updated":"2015-11-02 03:59:30.000000000","message":"You\u0027re right, I missed that one in my global search :(\n\nThat said, removing this and the part in keystone_domain, should work in theory :)","commit_id":"c1e75919063f23991757f93255d1ea80b3587007"},{"author":{"_account_id":6926,"name":"Bogdan Dobrelya","email":"bdobreli@redhat.com","username":"bogdando"},"change_message_id":"e198d22d45c85de521485274168c0c921e795188","unresolved":false,"context_lines":[{"line_number":49,"context_line":""},{"line_number":50,"context_line":"  def self.resource_to_name(domain, name, check_for_default\u003dtrue)"},{"line_number":51,"context_line":"    raise Puppet::Error, \"Domain cannot be nil for project \u0027#{name}\u0027. \" \\"},{"line_number":52,"context_line":"      \u0027Please report a bug.\u0027 if domain.nil?"},{"line_number":53,"context_line":"    join_str \u003d \u0027::\u0027"},{"line_number":54,"context_line":"    name_display \u003d [name]"},{"line_number":55,"context_line":"    unless check_for_default \u0026\u0026 domain \u003d\u003d default_domain"}],"source_content_type":"text/x-ruby","patch_set":43,"id":"1a26ad4f_47e2a230","line":52,"updated":"2015-11-02 14:22:38.000000000","message":"Why to test this here for the nil value? It looks not a common pattern for providers. And what if it would be an empty string or an integer? What if just to rescue the nil as an empty string, if \u0027\u0027 is acceptable?\n\nSuch things should be done in the munge section instead.","commit_id":"fa2638354ad9f6b7b27b6a0f539eceeb9bfe8210"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"63172159f19ea950d5167a5827e69db82c90c49a","unresolved":false,"context_lines":[{"line_number":49,"context_line":""},{"line_number":50,"context_line":"  def self.resource_to_name(domain, name, check_for_default\u003dtrue)"},{"line_number":51,"context_line":"    raise Puppet::Error, \"Domain cannot be nil for project \u0027#{name}\u0027. \" \\"},{"line_number":52,"context_line":"      \u0027Please report a bug.\u0027 if domain.nil?"},{"line_number":53,"context_line":"    join_str \u003d \u0027::\u0027"},{"line_number":54,"context_line":"    name_display \u003d [name]"},{"line_number":55,"context_line":"    unless check_for_default \u0026\u0026 domain \u003d\u003d default_domain"}],"source_content_type":"text/x-ruby","patch_set":43,"id":"1a26ad4f_c437834b","line":52,"in_reply_to":"1a26ad4f_47e2a230","updated":"2015-11-02 19:00:13.000000000","message":"It\u0027s really an internal function, so the \"nil\" test is only there in case some the domain from domain_from_id give back a nil value.  It is used in self.instance and prefetch to make a \"full\" name.  It is of no use in the munge function as it\u0027s self.title_patterns that take care of name munging in the composite namevar world.\n\nThe main problem is that the \"puppet resource\" only display properties.  As \"domain\" is a parameter, it is not displayed.  To work around that we modify the name to look like name::domain, so that the user have a chance to differentiate between the resource.\n\nDoes it make sense?","commit_id":"fa2638354ad9f6b7b27b6a0f539eceeb9bfe8210"}],"lib/puppet/provider/keystone_tenant/openstack.rb":[{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"032d6f7d82db72d0f05b2690565365ec93a2bb32","unresolved":false,"context_lines":[{"line_number":36,"context_line":"    @property_hash[:ensure] \u003d :present"},{"line_number":37,"context_line":"  rescue Puppet::ExecutionFailure \u003d\u003e e"},{"line_number":38,"context_line":"    if e.message \u003d~ /No domain with a name or ID of/"},{"line_number":39,"context_line":"      raise(Puppet::Error, \"No project #{resource[:name]} with domain #{resource[:domain]} found\")"},{"line_number":40,"context_line":"    else"},{"line_number":41,"context_line":"      raise"},{"line_number":42,"context_line":"    end"}],"source_content_type":"text/x-ruby","patch_set":23,"id":"5a2ca52d_2cffd502","line":39,"updated":"2015-10-13 14:58:22.000000000","message":"The error is a non existant domain.  It used to be catched by a fetch domain but as there is no guessing anymore about the domain name (either given or default) such method is not called anymore.  I catch it and re-raise it to have the same error message than before and not an openstack cli error.","commit_id":"148b7edcc73f36ed0572a083c33573715074e52b"},{"author":{"_account_id":9983,"name":"Richard Megginson","email":"rmeggins@redhat.com","username":"rmeggins"},"change_message_id":"d72bef06a1dab014a0b9fd47779009632e8f80eb","unresolved":false,"context_lines":[{"line_number":36,"context_line":"    @property_hash[:ensure] \u003d :present"},{"line_number":37,"context_line":"  rescue Puppet::ExecutionFailure \u003d\u003e e"},{"line_number":38,"context_line":"    if e.message \u003d~ /No domain with a name or ID of/"},{"line_number":39,"context_line":"      raise(Puppet::Error, \"No project #{resource[:name]} with domain #{resource[:domain]} found\")"},{"line_number":40,"context_line":"    else"},{"line_number":41,"context_line":"      raise"},{"line_number":42,"context_line":"    end"}],"source_content_type":"text/x-ruby","patch_set":23,"id":"7a2fa921_a0867a63","line":39,"updated":"2015-10-12 18:56:34.000000000","message":"Why do you have to rescue and re-raise this exception?  What is the error here?  That the domain does not exist?","commit_id":"148b7edcc73f36ed0572a083c33573715074e52b"}],"lib/puppet/provider/keystone_user/openstack.rb":[{"author":{"_account_id":9983,"name":"Richard Megginson","email":"rmeggins@redhat.com","username":"rmeggins"},"change_message_id":"d72bef06a1dab014a0b9fd47779009632e8f80eb","unresolved":false,"context_lines":[{"line_number":167,"context_line":"    project \u003d self.class.fetch_project(project_name, domain_name)"},{"line_number":168,"context_line":"    project_id \u003d project ? project[:id] : nil"},{"line_number":169,"context_line":"    unless project_id"},{"line_number":170,"context_line":"      raise(Puppet::Error, \"No project found for name #{project_name} and domain #{domain_name}\")"},{"line_number":171,"context_line":"    end"},{"line_number":172,"context_line":"    if project_id  \u003d\u003d :absent"},{"line_number":173,"context_line":"      raise(Puppet::Error, \"Project #{newproject} missing when creating user #{resource[:name]}\")"}],"source_content_type":"text/x-ruby","patch_set":23,"id":"7a2fa921_a0d43a47","line":170,"updated":"2015-10-12 18:56:34.000000000","message":"How about \"Project #{project_name} not found in domain #{domain_name} for user #{resource[:title]}\"?","commit_id":"148b7edcc73f36ed0572a083c33573715074e52b"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"032d6f7d82db72d0f05b2690565365ec93a2bb32","unresolved":false,"context_lines":[{"line_number":167,"context_line":"    project \u003d self.class.fetch_project(project_name, domain_name)"},{"line_number":168,"context_line":"    project_id \u003d project ? project[:id] : nil"},{"line_number":169,"context_line":"    unless project_id"},{"line_number":170,"context_line":"      raise(Puppet::Error, \"No project found for name #{project_name} and domain #{domain_name}\")"},{"line_number":171,"context_line":"    end"},{"line_number":172,"context_line":"    if project_id  \u003d\u003d :absent"},{"line_number":173,"context_line":"      raise(Puppet::Error, \"Project #{newproject} missing when creating user #{resource[:name]}\")"}],"source_content_type":"text/x-ruby","patch_set":23,"id":"5a2ca52d_ec08fd2a","line":170,"updated":"2015-10-13 14:58:22.000000000","message":"I just changed the variable name to match the tenant project and domain name.  It was giving the tenant name and the domain of the user without this change.  Furthermore, currently any problem with the domain will be raised by fetch_project, so this work only for faulty project_name.  As for the title it appears with standard puppet like this \"Keystone_user[new_admin]\".  So it\u0027s already there.\nAnyway, it goes away.","commit_id":"148b7edcc73f36ed0572a083c33573715074e52b"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"300a2e639b94c036c274be5db9058935863445c9","unresolved":false,"context_lines":[{"line_number":167,"context_line":"    project \u003d self.class.fetch_project(project_name, domain_name)"},{"line_number":168,"context_line":"    project_id \u003d project ? project[:id] : nil"},{"line_number":169,"context_line":"    unless project_id"},{"line_number":170,"context_line":"      raise(Puppet::Error, \"No project found for name #{project_name} and domain #{domain_name}\")"},{"line_number":171,"context_line":"    end"},{"line_number":172,"context_line":"    if project_id  \u003d\u003d :absent"},{"line_number":173,"context_line":"      raise(Puppet::Error, \"Project #{newproject} missing when creating user #{resource[:name]}\")"}],"source_content_type":"text/x-ruby","patch_set":23,"id":"7a2fa921_abe16f00","line":170,"in_reply_to":"7a2fa921_a0d43a47","updated":"2015-10-13 01:49:06.000000000","message":"This stuff is to go away with https://review.openstack.org/228744.\nTherefore I\u0027d suggest to not worry about it by adding a dependency to above.","commit_id":"148b7edcc73f36ed0572a083c33573715074e52b"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"7f8f7ae191d70fad7bc114c726b476586d695e4a","unresolved":false,"context_lines":[{"line_number":17,"context_line":"  end"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"  def create"},{"line_number":20,"context_line":"    # see if resource[:domain], or user specified as user::domain"},{"line_number":21,"context_line":"    user_name, user_domain \u003d resource[:name], resource[:domain]"},{"line_number":22,"context_line":"    properties \u003d [user_name]"},{"line_number":23,"context_line":"    if resource[:enabled] \u003d\u003d :true"}],"source_content_type":"text/x-ruby","patch_set":34,"id":"1a26ad4f_2bc80d55","line":20,"updated":"2015-10-28 07:55:02.000000000","message":"Comment not needed any more.","commit_id":"912827bdf1679d45bcff5408a578b4e3d420ea70"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"9ebea2266c92b2e0d915135f44ecff4d2cc51bc7","unresolved":false,"context_lines":[{"line_number":17,"context_line":"  end"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"  def create"},{"line_number":20,"context_line":"    # see if resource[:domain], or user specified as user::domain"},{"line_number":21,"context_line":"    user_name, user_domain \u003d resource[:name], resource[:domain]"},{"line_number":22,"context_line":"    properties \u003d [user_name]"},{"line_number":23,"context_line":"    if resource[:enabled] \u003d\u003d :true"}],"source_content_type":"text/x-ruby","patch_set":34,"id":"1a26ad4f_3e7837c4","line":20,"updated":"2015-10-28 18:28:58.000000000","message":"Done","commit_id":"912827bdf1679d45bcff5408a578b4e3d420ea70"}],"lib/puppet/type/keystone_user.rb":[{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"032d6f7d82db72d0f05b2690565365ec93a2bb32","unresolved":false,"context_lines":[{"line_number":84,"context_line":""},{"line_number":85,"context_line":"  newparam(:domain) do"},{"line_number":86,"context_line":"    isnamevar"},{"line_number":87,"context_line":"    defaultto PuppetX::Keystone::CompositeNamevar::Unset"},{"line_number":88,"context_line":"  end"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"  autorequire(:keystone_tenant) do"}],"source_content_type":"text/x-ruby","patch_set":23,"id":"5a2ca52d_0c0e9934","line":87,"updated":"2015-10-13 14:58:22.000000000","message":"@Richard: good catch.  The Unset symbol is required for user_role as project and domain are mutually exclusive.  On user and tenant, this feature is not used as there is no such ambiguity.  name and domain are always set.  I\u0027m going to set it to \u0027Default\u0027 here.\n@Gilles: I would rather not call an outside fonction from here and keep stuff simple.  I\u0027m not sure I get the \"might be different\".  Didn\u0027t we agree on having the default domain not calculated at all ?\n@all: After some tinkering on the Default domain, I\u0027m going to try to search the catalog to have a homogeneous behaviour across the keystone module where the default domain can be changed.  See my firste comment.","commit_id":"148b7edcc73f36ed0572a083c33573715074e52b"},{"author":{"_account_id":9983,"name":"Richard Megginson","email":"rmeggins@redhat.com","username":"rmeggins"},"change_message_id":"d72bef06a1dab014a0b9fd47779009632e8f80eb","unresolved":false,"context_lines":[{"line_number":84,"context_line":""},{"line_number":85,"context_line":"  newparam(:domain) do"},{"line_number":86,"context_line":"    isnamevar"},{"line_number":87,"context_line":"    defaultto PuppetX::Keystone::CompositeNamevar::Unset"},{"line_number":88,"context_line":"  end"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"  autorequire(:keystone_tenant) do"}],"source_content_type":"text/x-ruby","patch_set":23,"id":"7a2fa921_038978ca","line":87,"updated":"2015-10-12 18:56:34.000000000","message":"Why is the domain default \u0027Default\u0027 for tenant but not for user?","commit_id":"148b7edcc73f36ed0572a083c33573715074e52b"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"300a2e639b94c036c274be5db9058935863445c9","unresolved":false,"context_lines":[{"line_number":84,"context_line":""},{"line_number":85,"context_line":"  newparam(:domain) do"},{"line_number":86,"context_line":"    isnamevar"},{"line_number":87,"context_line":"    defaultto PuppetX::Keystone::CompositeNamevar::Unset"},{"line_number":88,"context_line":"  end"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"  autorequire(:keystone_tenant) do"}],"source_content_type":"text/x-ruby","patch_set":23,"id":"7a2fa921_6b511762","line":87,"in_reply_to":"7a2fa921_038978ca","updated":"2015-10-13 01:49:06.000000000","message":"Actually I think the default should be the default domain, which might be different from default if set.","commit_id":"148b7edcc73f36ed0572a083c33573715074e52b"}],"lib/puppet/type/keystone_user_role.rb":[{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"032d6f7d82db72d0f05b2690565365ec93a2bb32","unresolved":false,"context_lines":[{"line_number":58,"context_line":"    rv \u003d []"},{"line_number":59,"context_line":"    rv \u003c\u003c provider.class"},{"line_number":60,"context_line":"      .resource_to_name(self[:project_domain],"},{"line_number":61,"context_line":"                        self[:project]) unless self_is_set?(:domain)"},{"line_number":62,"context_line":"    rv"},{"line_number":63,"context_line":"  end"},{"line_number":64,"context_line":""}],"source_content_type":"text/x-ruby","patch_set":23,"id":"5a2ca52d_c9097333","line":61,"updated":"2015-10-13 14:58:22.000000000","message":"Done","commit_id":"148b7edcc73f36ed0572a083c33573715074e52b"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"300a2e639b94c036c274be5db9058935863445c9","unresolved":false,"context_lines":[{"line_number":58,"context_line":"    rv \u003d []"},{"line_number":59,"context_line":"    rv \u003c\u003c provider.class"},{"line_number":60,"context_line":"      .resource_to_name(self[:project_domain],"},{"line_number":61,"context_line":"                        self[:project]) unless self_is_set?(:domain)"},{"line_number":62,"context_line":"    rv"},{"line_number":63,"context_line":"  end"},{"line_number":64,"context_line":""}],"source_content_type":"text/x-ruby","patch_set":23,"id":"7a2fa921_2bf9df4d","line":61,"updated":"2015-10-13 01:49:06.000000000","message":"Standard: The unless block to wrap multi-lines.\nI understand it\u0027s one statement but it spreads over multiple lines.\nThe idea is to be easier to read.","commit_id":"148b7edcc73f36ed0572a083c33573715074e52b"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"032d6f7d82db72d0f05b2690565365ec93a2bb32","unresolved":false,"context_lines":[{"line_number":78,"context_line":"    [\u0027keystone_started\u0027]"},{"line_number":79,"context_line":"  end"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"  # \u0027heat_admin@::heat_stack\""},{"line_number":82,"context_line":"  # parse this: \u003cuser(::domain)?\u003e@\u003cproject(::domain)?|::domain\u003e"},{"line_number":83,"context_line":"  def self.title_patterns"},{"line_number":84,"context_line":"    user \u003d PuppetX::Keystone::CompositeNamevar.not_two_colon_regex"}],"source_content_type":"text/x-ruby","patch_set":23,"id":"5a2ca52d_69061f22","line":81,"updated":"2015-10-13 14:58:22.000000000","message":"Done","commit_id":"148b7edcc73f36ed0572a083c33573715074e52b"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"300a2e639b94c036c274be5db9058935863445c9","unresolved":false,"context_lines":[{"line_number":78,"context_line":"    [\u0027keystone_started\u0027]"},{"line_number":79,"context_line":"  end"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"  # \u0027heat_admin@::heat_stack\""},{"line_number":82,"context_line":"  # parse this: \u003cuser(::domain)?\u003e@\u003cproject(::domain)?|::domain\u003e"},{"line_number":83,"context_line":"  def self.title_patterns"},{"line_number":84,"context_line":"    user \u003d PuppetX::Keystone::CompositeNamevar.not_two_colon_regex"}],"source_content_type":"text/x-ruby","patch_set":23,"id":"7a2fa921_abbacf6c","line":81,"updated":"2015-10-13 01:49:06.000000000","message":"I think this hard coded comment could be avoided","commit_id":"148b7edcc73f36ed0572a083c33573715074e52b"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"032d6f7d82db72d0f05b2690565365ec93a2bb32","unresolved":false,"context_lines":[{"line_number":79,"context_line":"  end"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"  # \u0027heat_admin@::heat_stack\""},{"line_number":82,"context_line":"  # parse this: \u003cuser(::domain)?\u003e@\u003cproject(::domain)?|::domain\u003e"},{"line_number":83,"context_line":"  def self.title_patterns"},{"line_number":84,"context_line":"    user \u003d PuppetX::Keystone::CompositeNamevar.not_two_colon_regex"},{"line_number":85,"context_line":"    project_domain \u003d user"}],"source_content_type":"text/x-ruby","patch_set":23,"id":"5a2ca52d_89037b0f","line":82,"updated":"2015-10-13 14:58:22.000000000","message":"Done","commit_id":"148b7edcc73f36ed0572a083c33573715074e52b"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"300a2e639b94c036c274be5db9058935863445c9","unresolved":false,"context_lines":[{"line_number":79,"context_line":"  end"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"  # \u0027heat_admin@::heat_stack\""},{"line_number":82,"context_line":"  # parse this: \u003cuser(::domain)?\u003e@\u003cproject(::domain)?|::domain\u003e"},{"line_number":83,"context_line":"  def self.title_patterns"},{"line_number":84,"context_line":"    user \u003d PuppetX::Keystone::CompositeNamevar.not_two_colon_regex"},{"line_number":85,"context_line":"    project_domain \u003d user"}],"source_content_type":"text/x-ruby","patch_set":23,"id":"7a2fa921_ae0dfd38","line":82,"updated":"2015-10-13 01:49:06.000000000","message":"Wouldn\u0027t this comment be more useful in the desc (at the top)?\nI mean, we know what self.title_patterns is about and at the same time we could help people discovering the provider.","commit_id":"148b7edcc73f36ed0572a083c33573715074e52b"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"7f8f7ae191d70fad7bc114c726b476586d695e4a","unresolved":false,"context_lines":[{"line_number":79,"context_line":"      # Pass through title parsing for matching resource."},{"line_number":80,"context_line":"      rv \u003c\u003c provider.class"},{"line_number":81,"context_line":"        .resource_to_name(self[:project_domain],"},{"line_number":82,"context_line":"                          self[:project], false)"},{"line_number":83,"context_line":"    end"},{"line_number":84,"context_line":"    rv"},{"line_number":85,"context_line":"  end"}],"source_content_type":"text/x-ruby","patch_set":34,"id":"1a26ad4f_cb4e39c1","line":82,"updated":"2015-10-28 07:55:02.000000000","message":"Cosmetic, for readability (at least for me, but I think it\u0027s more compact too:\n\nrv \u003c\u003c provider.class.resource_to_name(self[:project_domain],\n  self[:project], false)","commit_id":"912827bdf1679d45bcff5408a578b4e3d420ea70"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"9ebea2266c92b2e0d915135f44ecff4d2cc51bc7","unresolved":false,"context_lines":[{"line_number":79,"context_line":"      # Pass through title parsing for matching resource."},{"line_number":80,"context_line":"      rv \u003c\u003c provider.class"},{"line_number":81,"context_line":"        .resource_to_name(self[:project_domain],"},{"line_number":82,"context_line":"                          self[:project], false)"},{"line_number":83,"context_line":"    end"},{"line_number":84,"context_line":"    rv"},{"line_number":85,"context_line":"  end"}],"source_content_type":"text/x-ruby","patch_set":34,"id":"1a26ad4f_9e72abe4","line":82,"updated":"2015-10-28 18:28:58.000000000","message":"Done","commit_id":"912827bdf1679d45bcff5408a578b4e3d420ea70"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"8e3fe6f15591caa3a72c5d37a830cd5bc3957a17","unresolved":false,"context_lines":[{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    which means the user is required.  Project and"},{"line_number":24,"context_line":"    domain are mutually exclusive.  User_domain and project_domain"},{"line_number":25,"context_line":"    are optional and default to \u0027Default\u0027 if unspecified."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"  EOT"},{"line_number":28,"context_line":""}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_138651b9","line":25,"updated":"2015-10-30 17:06:00.000000000","message":"No problem.  Your question are very welcome, especially the challanging one as they force me to be very clear on the why.  With a patch this size, it\u0027s easy to lost track of why you did what.","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"a17e08e52999f031b3ad37a70a99857f572f65ee","unresolved":false,"context_lines":[{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    which means the user is required.  Project and"},{"line_number":24,"context_line":"    domain are mutually exclusive.  User_domain and project_domain"},{"line_number":25,"context_line":"    are optional and default to \u0027Default\u0027 if unspecified."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"  EOT"},{"line_number":28,"context_line":""}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_6a02480d","line":25,"updated":"2015-10-29 20:02:09.000000000","message":"Oki, there are two issues here.\n\nFirst, this description follows the spec described here: https://review.openstack.org/#/c/219127/6/specs/kilo/api-v3-support.rst, which is \"if unspecified set to to Default\".  There is no reference to \"go and check the domain id in keystone.conf and get the domain name in the db\".  So, this is correct regarding the \"new\" spec, I think.\n\nSecond, I have added support for catalog parsing, and if the user is setting a default domain using the keystone_domain{\u0027blah\u0027: is_default\u003d\u003etrue} resource it will be taken into accout and default domain will be \u0027blah\u0027.  It follows that if the user is doing class {\u0027kestone\u0027: default_domain \u003d\u003e \u0027blah\u0027, ....} this will work as expected.  Note that this may be a bad idea as:\n 1. it has not been really discussed elsewhere than as a comment in the above review;\n 2. if the user is using domain and is bold enough to create small manifests for creating user in the default domain, this won\u0027t work as the keystone_domain{is_default} will be missing from this small manifest and \u0027Default\u0027 will be taken.\n\nAs we discussed a little already, the fact that we depend on keystone.conf and keystone database and API to be up and running only to get the full name of all the resources seems very wrong to me.  I may be biaised, as this whole composite namevar stuff cannot work if we do such thing.  We have to know the name of the resources before the catalog is applied, which is not possible if the name depends on the catalog to be applied.  Chicken/egg stuff.  I raised the point in the above link, and it seems ok at this time.\n\nDid I get the above review wrong?  What is the current stand on the \u0027Default\u0027 domain?\n\nIn the meantime I update the description to match the current behavior.","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"93934db269bc4119519d4c82acaad6b95472b744","unresolved":false,"context_lines":[{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    which means the user is required.  Project and"},{"line_number":24,"context_line":"    domain are mutually exclusive.  User_domain and project_domain"},{"line_number":25,"context_line":"    are optional and default to \u0027Default\u0027 if unspecified."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"  EOT"},{"line_number":28,"context_line":""}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_db826b00","line":25,"updated":"2015-10-29 03:08:08.000000000","message":"What about if \u0027Default\u0027 domain (id \u0027default\u0027) doesn\u0027t exist because the user has replaced it with something else?\n\nIt should default to whatever is the name of the default domain id.","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"d3806205a0afc256d4c52532176ecdfd47a770d0","unresolved":false,"context_lines":[{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    which means the user is required.  Project and"},{"line_number":24,"context_line":"    domain are mutually exclusive.  User_domain and project_domain"},{"line_number":25,"context_line":"    are optional and default to \u0027Default\u0027 if unspecified."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"  EOT"},{"line_number":28,"context_line":""}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_bf641b22","line":25,"in_reply_to":"1a26ad4f_6a02480d","updated":"2015-10-30 01:22:32.000000000","message":"Thanks for the explanation and sorry for the extra effor as I actually drafted a comment to my comment but forgot to publish it yesterday :(\n\nBottom line, it makes perfect sense to rely on the catalogue and not on keystone.conf or dynamic result from API.","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"}],"lib/puppet_x/keystone/composite_namevar.rb":[{"author":{"_account_id":7423,"name":"Cody Herriges","email":"cody@herriges.org","username":"ody"},"change_message_id":"2edab45bec8f86fbf0847a8e5ccbc923c639ba52","unresolved":false,"context_lines":[{"line_number":82,"context_line":""},{"line_number":83,"context_line":"# Cherry pick PUP-1073 from puppetlabs: support of composite namevar for alias."},{"line_number":84,"context_line":"if Gem::Version.new(Puppet.version) \u003c Gem::Version.new(\u00274.0.0\u0027)"},{"line_number":85,"context_line":"  Puppet::Resource::Catalog.class_eval do"},{"line_number":86,"context_line":"    def create_resource_aliases(resource)"},{"line_number":87,"context_line":"      # Skip creating aliases and checking collisions for non-isomorphic resources."},{"line_number":88,"context_line":"      return unless resource.respond_to?(:isomorphic?) and resource.isomorphic?"}],"source_content_type":"text/x-ruby","patch_set":25,"id":"5a2ca52d_62bf11d1","line":85,"updated":"2015-10-13 17:12:38.000000000","message":"If tests pass without this I\u0027d just not ship the monkey patch.  Composite namevars I know to have been working since 2.7, not just since PUP-1073.","commit_id":"f390fc00bea39ddc70e7f4d0a055217175996369"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"e52f0772167a89c7a94a408d4819b68f8030a168","unresolved":false,"context_lines":[{"line_number":82,"context_line":""},{"line_number":83,"context_line":"# Cherry pick PUP-1073 from puppetlabs: support of composite namevar for alias."},{"line_number":84,"context_line":"if Gem::Version.new(Puppet.version) \u003c Gem::Version.new(\u00274.0.0\u0027)"},{"line_number":85,"context_line":"  Puppet::Resource::Catalog.class_eval do"},{"line_number":86,"context_line":"    def create_resource_aliases(resource)"},{"line_number":87,"context_line":"      # Skip creating aliases and checking collisions for non-isomorphic resources."},{"line_number":88,"context_line":"      return unless resource.respond_to?(:isomorphic?) and resource.isomorphic?"}],"source_content_type":"text/x-ruby","patch_set":25,"id":"5a2ca52d_4a109e4e","line":85,"in_reply_to":"5a2ca52d_62bf11d1","updated":"2015-10-15 11:18:04.000000000","message":"Unfortunatly the tests do not pass without it.  I made this patch to\nshow you that the java_ks is affected by this problem\nhttps://github.com/puppetlabs/puppetlabs-java_ks/compare/master...sathlan:bug/resource_duplication_detection_broken_before_puppet_4 .  It shows that only a difference in the title is enought to fool puppet into believing that the two resources are differents.  When you try to apply this for real, the system will fail in unexpected ways as it will try to converge the same resource twice.\n\nAs a side note, it should be pointed that in those tests the resource tested has :title, :name, :target always defined by definition in @app_example_com, which may render the tests a bit non relevent.","commit_id":"f390fc00bea39ddc70e7f4d0a055217175996369"},{"author":{"_account_id":7423,"name":"Cody Herriges","email":"cody@herriges.org","username":"ody"},"change_message_id":"2edab45bec8f86fbf0847a8e5ccbc923c639ba52","unresolved":false,"context_lines":[{"line_number":95,"context_line":"    end"},{"line_number":96,"context_line":"  end"},{"line_number":97,"context_line":"  Puppet::Resource.class_eval do"},{"line_number":98,"context_line":"    def uniqueness_key"},{"line_number":99,"context_line":"      # Temporary kludge to deal with inconsistent use patterns; ensure we don\u0027t return nil for namevar/:name"},{"line_number":100,"context_line":"      h \u003d self.to_hash"},{"line_number":101,"context_line":"      name \u003d h[namevar] || h[:name] || self.name"}],"source_content_type":"text/x-ruby","patch_set":25,"id":"5a2ca52d_c2ddc571","line":98,"updated":"2015-10-13 17:12:38.000000000","message":"I don\u0027t think is a hard requirement, as long as you take into account that a person can create an arbitrary title in your patterns defined in title_patterns.\n\nhttps://github.com/puppetlabs/puppetlabs-java_ks/blob/master/lib/puppet/type/java_ks.rb#L152-L176\n\nThat first pattern in the snippet I posted accounts for always capturing :name and so avoiding ever triggering this.","commit_id":"f390fc00bea39ddc70e7f4d0a055217175996369"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"e52f0772167a89c7a94a408d4819b68f8030a168","unresolved":false,"context_lines":[{"line_number":95,"context_line":"    end"},{"line_number":96,"context_line":"  end"},{"line_number":97,"context_line":"  Puppet::Resource.class_eval do"},{"line_number":98,"context_line":"    def uniqueness_key"},{"line_number":99,"context_line":"      # Temporary kludge to deal with inconsistent use patterns; ensure we don\u0027t return nil for namevar/:name"},{"line_number":100,"context_line":"      h \u003d self.to_hash"},{"line_number":101,"context_line":"      name \u003d h[namevar] || h[:name] || self.name"}],"source_content_type":"text/x-ruby","patch_set":25,"id":"5a2ca52d_cf4fd7bb","line":98,"in_reply_to":"5a2ca52d_c2ddc571","updated":"2015-10-15 11:18:04.000000000","message":"This is not the case actually.  If you check the title_patterns function in patchset #18, the name is always filled for Keystone_tenant, but I had the problem with this:\n\n    keystone_tenant { \u0027servicesv3\u0027:\n      ensure      \u003d\u003e present,\n      enabled     \u003d\u003e true,\n      description \u003d\u003e \u0027Tenant for the openstack services\u0027,\n      domain      \u003d\u003e \u0027admin_domain\u0027,\n    }\n    keystone_tenant { \u0027openstackv3\u0027:\n      ensure      \u003d\u003e present,\n      enabled     \u003d\u003e true,\n      description \u003d\u003e \u0027admin tenant\u0027,\n      domain      \u003d\u003e \u0027admin_domain\u0027,\n    }\n\nThose two tenants were detected as the same resource with a cannot alias error (http://logs.openstack.org/19/226919/18/check/gate-puppet-keystone-puppet-beaker-rspec-dsvm-trusty/1bc70f3/console.html)\n\nThe problem is puzzling :)  After investigation, it appears that there is a first round of resources addition to the catalog, not using Puppet::Type::\u003cType\u003e as we test in the unit/type rspec, but using Puppet::Parser::Resource.  This round comes first.  Those objects do not call the title_patterns as they are not \"Puppet::Type\" class.  Then for puppet at this moment, the uniqueness_key method returns [\u0027admin_domain\u0027, nil] : title is filled, domain is filled, name is nil, and title hasn\u0027t been parsed.  So in this first round the two resources above are the same, making puppet fail.\n\nIncluding this second part of the PUP-1073 patch, make it works even at this stage as \"self.name\" make the two resources differents.\n\nIt\u0027s quite complicated to test this in rspec unit/type as you have to create :\n- a node object;\n- a compiler object;\n- a scope object;\n- puppet::parser::resource objects which are themselve made of Puppet::Parser::Resource::Param object\n\ntoo many things to create.  This is best captured with beaker.","commit_id":"f390fc00bea39ddc70e7f4d0a055217175996369"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"c375e58da3c724bc36e9f822c4a24ddccd7f8cc7","unresolved":false,"context_lines":[{"line_number":56,"context_line":"            r.class.to_s \u003d\u003d \u0027Puppet::Type::Keystone_domain\u0027 \u0026\u0026"},{"line_number":57,"context_line":"              r[:is_default] \u003d\u003d :true"},{"line_number":58,"context_line":"          end"},{"line_number":59,"context_line":"          current_default \u003d default_res_in_cat[:name] rescue \u0027Default\u0027"},{"line_number":60,"context_line":"          resource.provider.class.default_domain \u003d current_default"},{"line_number":61,"context_line":"        end"},{"line_number":62,"context_line":"        current_default"}],"source_content_type":"text/x-ruby","patch_set":27,"id":"5a2ca52d_780d2b36","line":59,"updated":"2015-10-15 11:39:17.000000000","message":"Because default_res_in_cat may be nil and nil[] raise an exception, not false.  It\u0027s a safe shortcut in this case.","commit_id":"e8eb8710e2c0075ddc5eb300e1b03472de1eb337"},{"author":{"_account_id":9983,"name":"Richard Megginson","email":"rmeggins@redhat.com","username":"rmeggins"},"change_message_id":"ae21102dc21a5d1783517801567c7f85c1ebea18","unresolved":false,"context_lines":[{"line_number":56,"context_line":"            r.class.to_s \u003d\u003d \u0027Puppet::Type::Keystone_domain\u0027 \u0026\u0026"},{"line_number":57,"context_line":"              r[:is_default] \u003d\u003d :true"},{"line_number":58,"context_line":"          end"},{"line_number":59,"context_line":"          current_default \u003d default_res_in_cat[:name] rescue \u0027Default\u0027"},{"line_number":60,"context_line":"          resource.provider.class.default_domain \u003d current_default"},{"line_number":61,"context_line":"        end"},{"line_number":62,"context_line":"        current_default"}],"source_content_type":"text/x-ruby","patch_set":27,"id":"5a2ca52d_7e968a4d","line":59,"updated":"2015-10-15 01:04:31.000000000","message":"why use `rescue` here instead of `||`?","commit_id":"e8eb8710e2c0075ddc5eb300e1b03472de1eb337"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"9ebea2266c92b2e0d915135f44ecff4d2cc51bc7","unresolved":false,"context_lines":[{"line_number":4,"context_line":""},{"line_number":5,"context_line":"      class Unset; end"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"      module Helpers"},{"line_number":8,"context_line":"        def is_set?(param, *args)"},{"line_number":9,"context_line":"          value \u003d nil"},{"line_number":10,"context_line":"          if args.empty?"}],"source_content_type":"text/x-ruby","patch_set":34,"id":"1a26ad4f_5e75f3c8","line":7,"updated":"2015-10-28 18:28:58.000000000","message":"Done","commit_id":"912827bdf1679d45bcff5408a578b4e3d420ea70"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"7f8f7ae191d70fad7bc114c726b476586d695e4a","unresolved":false,"context_lines":[{"line_number":4,"context_line":""},{"line_number":5,"context_line":"      class Unset; end"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"      module Helpers"},{"line_number":8,"context_line":"        def is_set?(param, *args)"},{"line_number":9,"context_line":"          value \u003d nil"},{"line_number":10,"context_line":"          if args.empty?"}],"source_content_type":"text/x-ruby","patch_set":34,"id":"1a26ad4f_5aec35c7","line":7,"updated":"2015-10-28 07:55:02.000000000","message":"I believe a dedicated file puppet_x/keystone/composite_namevar/helpers.rb to hold this.","commit_id":"912827bdf1679d45bcff5408a578b4e3d420ea70"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"9ebea2266c92b2e0d915135f44ecff4d2cc51bc7","unresolved":false,"context_lines":[{"line_number":5,"context_line":"      class Unset; end"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"      module Helpers"},{"line_number":8,"context_line":"        def is_set?(param, *args)"},{"line_number":9,"context_line":"          value \u003d nil"},{"line_number":10,"context_line":"          if args.empty?"},{"line_number":11,"context_line":"            value \u003d send(param.to_sym)"}],"source_content_type":"text/x-ruby","patch_set":34,"id":"1a26ad4f_fe815f90","line":8,"updated":"2015-10-28 18:28:58.000000000","message":"Done. It was only a way to say \"with any number of options.  But you right, in the end, I only use the first arg.","commit_id":"912827bdf1679d45bcff5408a578b4e3d420ea70"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"7f8f7ae191d70fad7bc114c726b476586d695e4a","unresolved":false,"context_lines":[{"line_number":5,"context_line":"      class Unset; end"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"      module Helpers"},{"line_number":8,"context_line":"        def is_set?(param, *args)"},{"line_number":9,"context_line":"          value \u003d nil"},{"line_number":10,"context_line":"          if args.empty?"},{"line_number":11,"context_line":"            value \u003d send(param.to_sym)"}],"source_content_type":"text/x-ruby","patch_set":34,"id":"1a26ad4f_a0bbdcc3","line":8,"updated":"2015-10-28 07:55:02.000000000","message":"Why an array here?  I don\u0027t see the need for it?","commit_id":"912827bdf1679d45bcff5408a578b4e3d420ea70"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"7f8f7ae191d70fad7bc114c726b476586d695e4a","unresolved":false,"context_lines":[{"line_number":15,"context_line":"          value !\u003d PuppetX::Keystone::CompositeNamevar::Unset"},{"line_number":16,"context_line":"        end"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"        def self_is_set?(key)"},{"line_number":19,"context_line":"          is_set?(:\u0027[]\u0027, key.to_sym)"},{"line_number":20,"context_line":"        end"},{"line_number":21,"context_line":""}],"source_content_type":"text/x-ruby","patch_set":34,"id":"1a26ad4f_a0367c01","line":18,"updated":"2015-10-28 07:55:02.000000000","message":"I think the name \u0027self_is_set?\u0027 is a bit confusing. \nCould we try something else? Maybe \u0027is_resource_set?\u0027","commit_id":"912827bdf1679d45bcff5408a578b4e3d420ea70"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"9ebea2266c92b2e0d915135f44ecff4d2cc51bc7","unresolved":false,"context_lines":[{"line_number":15,"context_line":"          value !\u003d PuppetX::Keystone::CompositeNamevar::Unset"},{"line_number":16,"context_line":"        end"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"        def self_is_set?(key)"},{"line_number":19,"context_line":"          is_set?(:\u0027[]\u0027, key.to_sym)"},{"line_number":20,"context_line":"        end"},{"line_number":21,"context_line":""}],"source_content_type":"text/x-ruby","patch_set":34,"id":"1a26ad4f_1e7f7bab","line":18,"updated":"2015-10-28 18:28:58.000000000","message":"is_resource_set is not appropriate as it\u0027s related to parameter, not resource.  What do you think of parameter_set?","commit_id":"912827bdf1679d45bcff5408a578b4e3d420ea70"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"ce386907c987ffff338d387dcbd1641b1b232ac3","unresolved":false,"context_lines":[{"line_number":15,"context_line":"          value !\u003d PuppetX::Keystone::CompositeNamevar::Unset"},{"line_number":16,"context_line":"        end"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"        def self_is_set?(key)"},{"line_number":19,"context_line":"          is_set?(:\u0027[]\u0027, key.to_sym)"},{"line_number":20,"context_line":"        end"},{"line_number":21,"context_line":""}],"source_content_type":"text/x-ruby","patch_set":34,"id":"1a26ad4f_1c40dea4","line":18,"in_reply_to":"1a26ad4f_1e7f7bab","updated":"2015-10-29 00:43:40.000000000","message":"parameter_set? looks much better :)","commit_id":"912827bdf1679d45bcff5408a578b4e3d420ea70"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"7f8f7ae191d70fad7bc114c726b476586d695e4a","unresolved":false,"context_lines":[{"line_number":20,"context_line":"        end"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"        def self.included(klass)"},{"line_number":23,"context_line":"          klass.extend Utilities if klass.to_s.match(/ProviderOpenstack$/)"},{"line_number":24,"context_line":"        end"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"        module Utilities"}],"source_content_type":"text/x-ruby","patch_set":34,"id":"1a26ad4f_80dd8016","line":23,"updated":"2015-10-28 07:55:02.000000000","message":"Do we need #prefetch_composite(resources) as both a class and instance methods?","commit_id":"912827bdf1679d45bcff5408a578b4e3d420ea70"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"9ebea2266c92b2e0d915135f44ecff4d2cc51bc7","unresolved":false,"context_lines":[{"line_number":20,"context_line":"        end"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"        def self.included(klass)"},{"line_number":23,"context_line":"          klass.extend Utilities if klass.to_s.match(/ProviderOpenstack$/)"},{"line_number":24,"context_line":"        end"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"        module Utilities"}],"source_content_type":"text/x-ruby","patch_set":34,"id":"1a26ad4f_be4a0782","line":23,"updated":"2015-10-28 18:28:58.000000000","message":"Not sure I get it.  This idiom enable to do a mixin in the class, adding method class.  So prefetch_composite is _only_ a class method.  This is required to be able to call it from from self.prefetch.  Do it clarify the point ?","commit_id":"912827bdf1679d45bcff5408a578b4e3d420ea70"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"ce386907c987ffff338d387dcbd1641b1b232ac3","unresolved":false,"context_lines":[{"line_number":20,"context_line":"        end"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"        def self.included(klass)"},{"line_number":23,"context_line":"          klass.extend Utilities if klass.to_s.match(/ProviderOpenstack$/)"},{"line_number":24,"context_line":"        end"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"        module Utilities"}],"source_content_type":"text/x-ruby","patch_set":34,"id":"1a26ad4f_5c35f643","line":23,"in_reply_to":"1a26ad4f_be4a0782","updated":"2015-10-29 00:43:40.000000000","message":"Yes and no. I understand the idiom it\u0027s just that modules\u0027 methods don\u0027t have to be defined as class methods because when used with include there are instance method and with extend they become defacto class methods.","commit_id":"912827bdf1679d45bcff5408a578b4e3d420ea70"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"9ebea2266c92b2e0d915135f44ecff4d2cc51bc7","unresolved":false,"context_lines":[{"line_number":23,"context_line":"          klass.extend Utilities if klass.to_s.match(/ProviderOpenstack$/)"},{"line_number":24,"context_line":"        end"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"        module Utilities"},{"line_number":27,"context_line":"          def prefetch_composite(resources)"},{"line_number":28,"context_line":"            # cannot trust puppet for correct resources with semantic title"},{"line_number":29,"context_line":"            res \u003d resources.values.first"}],"source_content_type":"text/x-ruby","patch_set":34,"id":"1a26ad4f_de4fc390","line":26,"updated":"2015-10-28 18:28:58.000000000","message":"Done","commit_id":"912827bdf1679d45bcff5408a578b4e3d420ea70"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"7f8f7ae191d70fad7bc114c726b476586d695e4a","unresolved":false,"context_lines":[{"line_number":23,"context_line":"          klass.extend Utilities if klass.to_s.match(/ProviderOpenstack$/)"},{"line_number":24,"context_line":"        end"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"        module Utilities"},{"line_number":27,"context_line":"          def prefetch_composite(resources)"},{"line_number":28,"context_line":"            # cannot trust puppet for correct resources with semantic title"},{"line_number":29,"context_line":"            res \u003d resources.values.first"}],"source_content_type":"text/x-ruby","patch_set":34,"id":"1a26ad4f_9a096d98","line":26,"updated":"2015-10-28 07:55:02.000000000","message":"In theory another file for that one too:\npuppet_x/keystone/composite_namevar/helpers/utilities.rb","commit_id":"912827bdf1679d45bcff5408a578b4e3d420ea70"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"7f8f7ae191d70fad7bc114c726b476586d695e4a","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        end"},{"line_number":50,"context_line":"      end"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"      def self.defaultto(resource)"},{"line_number":53,"context_line":"        current_default \u003d resource.provider.class.class_variable_get(\u0027@@default_domain\u0027)"},{"line_number":54,"context_line":"        if current_default.nil?"},{"line_number":55,"context_line":"          default_res_in_cat \u003d resource.catalog.resources.find do |r|"}],"source_content_type":"text/x-ruby","patch_set":34,"id":"1a26ad4f_ba81c991","line":52,"updated":"2015-10-28 07:55:02.000000000","message":"Maybe that\u0027s silly (I haven\u0027t spent enough on the type\u0027s structure) or you have already looked at it:\nWould it be possible to use an instance method here (and same for the following ones)?\nBy either overriding the type\u0027s instance method and if not possible then extend PuppetX::Keystone::CompositeNamevar module?","commit_id":"912827bdf1679d45bcff5408a578b4e3d420ea70"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"9ebea2266c92b2e0d915135f44ecff4d2cc51bc7","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        end"},{"line_number":50,"context_line":"      end"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"      def self.defaultto(resource)"},{"line_number":53,"context_line":"        current_default \u003d resource.provider.class.class_variable_get(\u0027@@default_domain\u0027)"},{"line_number":54,"context_line":"        if current_default.nil?"},{"line_number":55,"context_line":"          default_res_in_cat \u003d resource.catalog.resources.find do |r|"}],"source_content_type":"text/x-ruby","patch_set":34,"id":"1a26ad4f_7e446f73","line":52,"updated":"2015-10-28 18:28:58.000000000","message":"instance method would be possible, but then I will have to instantiate it, which makes no sense as there is no state to be kept here.  But, now that I look at it, it\u0027s really not composite namevar related, but only keystone domain related.  I put it there as a convenience as there is no equivalent of keystone.rb for type.  I will add one for stuff that are common in keystone type.","commit_id":"912827bdf1679d45bcff5408a578b4e3d420ea70"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"ce386907c987ffff338d387dcbd1641b1b232ac3","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        end"},{"line_number":50,"context_line":"      end"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"      def self.defaultto(resource)"},{"line_number":53,"context_line":"        current_default \u003d resource.provider.class.class_variable_get(\u0027@@default_domain\u0027)"},{"line_number":54,"context_line":"        if current_default.nil?"},{"line_number":55,"context_line":"          default_res_in_cat \u003d resource.catalog.resources.find do |r|"}],"source_content_type":"text/x-ruby","patch_set":34,"id":"1a26ad4f_3ccfda57","line":52,"in_reply_to":"1a26ad4f_7e446f73","updated":"2015-10-29 00:43:40.000000000","message":"Sounds good.","commit_id":"912827bdf1679d45bcff5408a578b4e3d420ea70"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"7f8f7ae191d70fad7bc114c726b476586d695e4a","unresolved":false,"context_lines":[{"line_number":52,"context_line":"      def self.defaultto(resource)"},{"line_number":53,"context_line":"        current_default \u003d resource.provider.class.class_variable_get(\u0027@@default_domain\u0027)"},{"line_number":54,"context_line":"        if current_default.nil?"},{"line_number":55,"context_line":"          default_res_in_cat \u003d resource.catalog.resources.find do |r|"},{"line_number":56,"context_line":"            r.class.to_s \u003d\u003d \u0027Puppet::Type::Keystone_domain\u0027 \u0026\u0026"},{"line_number":57,"context_line":"              r[:is_default] \u003d\u003d :true"},{"line_number":58,"context_line":"          end"}],"source_content_type":"text/x-ruby","patch_set":34,"id":"1a26ad4f_9abbadc4","line":55,"updated":"2015-10-28 07:55:02.000000000","message":"It\u0027s a detail and only a local variable but for readability could we use \u0027default\u0027 instead of \u0027default_res_in_cat\u0027?","commit_id":"912827bdf1679d45bcff5408a578b4e3d420ea70"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"9ebea2266c92b2e0d915135f44ecff4d2cc51bc7","unresolved":false,"context_lines":[{"line_number":52,"context_line":"      def self.defaultto(resource)"},{"line_number":53,"context_line":"        current_default \u003d resource.provider.class.class_variable_get(\u0027@@default_domain\u0027)"},{"line_number":54,"context_line":"        if current_default.nil?"},{"line_number":55,"context_line":"          default_res_in_cat \u003d resource.catalog.resources.find do |r|"},{"line_number":56,"context_line":"            r.class.to_s \u003d\u003d \u0027Puppet::Type::Keystone_domain\u0027 \u0026\u0026"},{"line_number":57,"context_line":"              r[:is_default] \u003d\u003d :true"},{"line_number":58,"context_line":"          end"}],"source_content_type":"text/x-ruby","patch_set":34,"id":"1a26ad4f_9e494b8e","line":55,"updated":"2015-10-28 18:28:58.000000000","message":"Well it helps me to follow where I am: is that the resource on the system or the RESourse IN the CATalog.  But that may be only me.  If you insist I\u0027ll change :)","commit_id":"912827bdf1679d45bcff5408a578b4e3d420ea70"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"ce386907c987ffff338d387dcbd1641b1b232ac3","unresolved":false,"context_lines":[{"line_number":52,"context_line":"      def self.defaultto(resource)"},{"line_number":53,"context_line":"        current_default \u003d resource.provider.class.class_variable_get(\u0027@@default_domain\u0027)"},{"line_number":54,"context_line":"        if current_default.nil?"},{"line_number":55,"context_line":"          default_res_in_cat \u003d resource.catalog.resources.find do |r|"},{"line_number":56,"context_line":"            r.class.to_s \u003d\u003d \u0027Puppet::Type::Keystone_domain\u0027 \u0026\u0026"},{"line_number":57,"context_line":"              r[:is_default] \u003d\u003d :true"},{"line_number":58,"context_line":"          end"}],"source_content_type":"text/x-ruby","patch_set":34,"id":"1a26ad4f_fcc1622f","line":55,"in_reply_to":"1a26ad4f_9e494b8e","updated":"2015-10-29 00:43:40.000000000","message":"I\u0027m not insisting!!! ;)","commit_id":"912827bdf1679d45bcff5408a578b4e3d420ea70"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"9ebea2266c92b2e0d915135f44ecff4d2cc51bc7","unresolved":false,"context_lines":[{"line_number":63,"context_line":"      end"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"      def self.not_two_colon_regex"},{"line_number":66,"context_line":"        # anything that has not 2 consecutive colons"},{"line_number":67,"context_line":"        Regexp.new(/(?:[^:]|:[^:])+/)"},{"line_number":68,"context_line":"      end"},{"line_number":69,"context_line":""}],"source_content_type":"text/x-ruby","patch_set":34,"id":"1a26ad4f_3e5fd742","line":66,"updated":"2015-10-28 18:28:58.000000000","message":"Done.","commit_id":"912827bdf1679d45bcff5408a578b4e3d420ea70"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"7f8f7ae191d70fad7bc114c726b476586d695e4a","unresolved":false,"context_lines":[{"line_number":63,"context_line":"      end"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"      def self.not_two_colon_regex"},{"line_number":66,"context_line":"        # anything that has not 2 consecutive colons"},{"line_number":67,"context_line":"        Regexp.new(/(?:[^:]|:[^:])+/)"},{"line_number":68,"context_line":"      end"},{"line_number":69,"context_line":""}],"source_content_type":"text/x-ruby","patch_set":34,"id":"1a26ad4f_5a40b556","line":66,"updated":"2015-10-28 07:55:02.000000000","message":"I\u0027d say:\n# Anything but 2 consecutive colons","commit_id":"912827bdf1679d45bcff5408a578b4e3d420ea70"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"9ebea2266c92b2e0d915135f44ecff4d2cc51bc7","unresolved":false,"context_lines":[{"line_number":91,"context_line":"  end"},{"line_number":92,"context_line":"end"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"# Cherry pick PUP-1073 from puppetlabs: support of composite namevar for alias."},{"line_number":95,"context_line":"if Gem::Version.new(Puppet.version) \u003c Gem::Version.new(\u00274.0.0\u0027)"},{"line_number":96,"context_line":"  Puppet::Resource::Catalog.class_eval do"},{"line_number":97,"context_line":"    def create_resource_aliases(resource)"}],"source_content_type":"text/x-ruby","patch_set":34,"id":"1a26ad4f_5e5c134b","line":94,"updated":"2015-10-28 18:28:58.000000000","message":"Done","commit_id":"912827bdf1679d45bcff5408a578b4e3d420ea70"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"7f8f7ae191d70fad7bc114c726b476586d695e4a","unresolved":false,"context_lines":[{"line_number":91,"context_line":"  end"},{"line_number":92,"context_line":"end"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"# Cherry pick PUP-1073 from puppetlabs: support of composite namevar for alias."},{"line_number":95,"context_line":"if Gem::Version.new(Puppet.version) \u003c Gem::Version.new(\u00274.0.0\u0027)"},{"line_number":96,"context_line":"  Puppet::Resource::Catalog.class_eval do"},{"line_number":97,"context_line":"    def create_resource_aliases(resource)"}],"source_content_type":"text/x-ruby","patch_set":34,"id":"1a26ad4f_3a8379ce","line":94,"updated":"2015-10-28 07:55:02.000000000","message":"I think this owes to be located on the top of the file because this is obviously important enough but although to make sure it\u0027s executed before anything else.","commit_id":"912827bdf1679d45bcff5408a578b4e3d420ea70"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"a17e08e52999f031b3ad37a70a99857f572f65ee","unresolved":false,"context_lines":[{"line_number":24,"context_line":"  end"},{"line_number":25,"context_line":"end"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"require \u0027puppet_x/keystone/composite_namevar/helpers\u0027"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"module PuppetX"},{"line_number":30,"context_line":"  module Keystone"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_451e5f64","line":27,"updated":"2015-10-29 20:02:09.000000000","message":"So that the user don\u0027t have to force the user to require \u0027composite_namevar\u0027 and \u0027composite_namevar/helpers\u0027.","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"93934db269bc4119519d4c82acaad6b95472b744","unresolved":false,"context_lines":[{"line_number":24,"context_line":"  end"},{"line_number":25,"context_line":"end"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"require \u0027puppet_x/keystone/composite_namevar/helpers\u0027"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"module PuppetX"},{"line_number":30,"context_line":"  module Keystone"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_fb7167a9","line":27,"updated":"2015-10-29 03:08:08.000000000","message":"Why is helpers required here?","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"d3806205a0afc256d4c52532176ecdfd47a770d0","unresolved":false,"context_lines":[{"line_number":24,"context_line":"  end"},{"line_number":25,"context_line":"end"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"require \u0027puppet_x/keystone/composite_namevar/helpers\u0027"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"module PuppetX"},{"line_number":30,"context_line":"  module Keystone"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_a25c04ee","line":27,"in_reply_to":"1a26ad4f_451e5f64","updated":"2015-10-30 01:22:32.000000000","message":"Ok","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"a17e08e52999f031b3ad37a70a99857f572f65ee","unresolved":false,"context_lines":[{"line_number":66,"context_line":"          ]"},{"line_number":67,"context_line":"        ]"},{"line_number":68,"context_line":"      end"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    end"},{"line_number":71,"context_line":"  end"},{"line_number":72,"context_line":"end"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_65191b79","line":69,"updated":"2015-10-29 20:02:09.000000000","message":"Done","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"93934db269bc4119519d4c82acaad6b95472b744","unresolved":false,"context_lines":[{"line_number":66,"context_line":"          ]"},{"line_number":67,"context_line":"        ]"},{"line_number":68,"context_line":"      end"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    end"},{"line_number":71,"context_line":"  end"},{"line_number":72,"context_line":"end"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_bb644fe5","line":69,"updated":"2015-10-29 03:08:08.000000000","message":"No LF here.","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"}],"lib/puppet_x/keystone/composite_namevar/helpers.rb":[{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"a17e08e52999f031b3ad37a70a99857f572f65ee","unresolved":false,"context_lines":[{"line_number":19,"context_line":"        end"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"        def self.included(klass)"},{"line_number":22,"context_line":"          klass.extend Utilities if klass.to_s.match(/ProviderOpenstack$/)"},{"line_number":23,"context_line":"        end"},{"line_number":24,"context_line":"      end"},{"line_number":25,"context_line":"    end"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_252a437f","line":22,"updated":"2015-10-29 20:02:09.000000000","message":"Arghh, you\u0027re putting my ruby fu to the test ... I\u0027ll try to defend my stance here :)\n\nI\u0027m discarding both :)  Let me explain myself.\n\nI do two things here (which may be bad but eventually seemed natural):\n 1. I add methods for the type and for the provider: set? and parameter_set?  set? is for provider, parameter_set? is for type.  I shouldn\u0027t make them a class method as:\n    a) set? is called in autorequire, which is called in the context of an *instance* of Puppet::Type::Keystone_user_role (for instance). \n    b) parameter? is called in instance method of the provider.\n    When I do send(param.to_sym) for instance, I expect to be in the instance of the provider and when I do [:param] in the type I expect to be in an instance of the type.  So they really are instance methods.  If I make them a class method as you suggest then I must add another parameter to pass \"self\" so that they know on which instance they are working.  This would look strange.\n 2. I add a class method to the provider: adding class method can be done in 3 ways in ruby (I think):\n    a) inheritence;\n    b) the metaprograming idiom I\u0027m using here (which is extend in disguise)\n    c) the extend call;\n\nI won\u0027t discuss why not inheritence as you didn\u0027t raise the subject.  \n\nNow, if I do extend, then as set? and parameter_set? are instance methods, I need the user in the provider class to do:\n  \n  include \u0027...Composite_namevar::Helpers\u0027\n  extand \u0027...Composite_namevar::Helpers::Utilities\u0027\n\nso that set? becomes an instance method and prefetch becomes a class method.  So this doesn\u0027t solve the \"one more file\" problem anyway and add some heavyness to the interface.\n\nSo I think the first proposition is missing the fact that set? and parameter_set? _are_ instance methods.\n\nFor the second proposition I think that it wouldn\u0027t nice to not use ruby functionality here and it has the same problem as the first proposition, as the premises are wrong here as well (I think)\n\nFrom my point of view, the best way to get ride of this utilities file would be for the utilities module to be defined inside the helpers file as it only exist as a way to extend the class instance (that\u0027s when metaprogramming becomes confusing :)) when included by a provider.\n\nHope I stand my ground honorably :)","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"93934db269bc4119519d4c82acaad6b95472b744","unresolved":false,"context_lines":[{"line_number":19,"context_line":"        end"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"        def self.included(klass)"},{"line_number":22,"context_line":"          klass.extend Utilities if klass.to_s.match(/ProviderOpenstack$/)"},{"line_number":23,"context_line":"        end"},{"line_number":24,"context_line":"      end"},{"line_number":25,"context_line":"    end"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_3b035f0f","line":22,"updated":"2015-10-29 03:08:08.000000000","message":"Following up from path set 34.\n\nFirst off, I understand there are many ways to achieve this, just adding ideas.\n\nI believe we could go away without adding the \u0027utilities.rb\u0027 library.\nThe main driver here is why \u0027set?\u0027 and \u0027parameter_set?\u0027 are instance methods? After all, they all are class level methods, including prefetch_compositie, as they\u0027re not within any instance object scope.\n\nSo what about this:\n\n1) Use \u0027extend\u0027 instead of \u0027include\u0027\n\nmodule PuppetX\n  module Keystone\n    module CompositeNamevar\n      module Helpers\n        def set?(param, argument \u003d nil)\n          ...\n        end\n\n        def parameter_set?(key)\n          ...\n        end\n        \n        def prefetch_composite(resources)\n          ...\n        end\n      end\n    end\n  end\nend\n\nThe module is mixed in using \u0027extend\u0027.\n\nCalls:  \nself.class.prefetch_composite\n\nor \n\n2) Use pure class methods calls without mixing in:\n\nmodule PuppetX\n  module Keystone\n    module CompositeNamevar\n      module Helpers\n        def self.set?(param, argument \u003d nil)\n          ...\n        end\n\n        def self.parameter_set?(key)\n          ...\n        end\n        \n        def self.prefetch_composite(resources)\n          ...\n        end\n      end\n    end\n  end\nend\n\nPuppetX::Keystone::CompositeNamevar::Helpers.prefetch_composite\nNo need to include/extend but makes the calls \u0027heavier\u0027.","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"8e3fe6f15591caa3a72c5d37a830cd5bc3957a17","unresolved":false,"context_lines":[{"line_number":19,"context_line":"        end"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"        def self.included(klass)"},{"line_number":22,"context_line":"          klass.extend Utilities if klass.to_s.match(/ProviderOpenstack$/)"},{"line_number":23,"context_line":"        end"},{"line_number":24,"context_line":"      end"},{"line_number":25,"context_line":"    end"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_d38f79ca","line":22,"updated":"2015-10-30 17:06:00.000000000","message":"set? and parameter_set? manipulate instance data.  They don\u0027t change it but access it.\n\nIn set? I send sym to an *instance* of Puppet::Type::Keystone_user_role::ProviderOpenstack or an instance of Puppet::Type::Keystone_user_role (it\u0027s the only place it\u0027s used currently, I think)\nIn parameter_set? is acting on an instance of Puppet::Type::Keystone_user_role.\n\nIf I make them class method, then I will have to pass \"self\" as a parameter (like in python...)\n\nAll in all with this premise false (there are accessing instance data, through instance method call), the rest doesn\u0027t stand.\n\nI may be missing something, as I put this in my previous comment already.","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"d3806205a0afc256d4c52532176ecdfd47a770d0","unresolved":false,"context_lines":[{"line_number":19,"context_line":"        end"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"        def self.included(klass)"},{"line_number":22,"context_line":"          klass.extend Utilities if klass.to_s.match(/ProviderOpenstack$/)"},{"line_number":23,"context_line":"        end"},{"line_number":24,"context_line":"      end"},{"line_number":25,"context_line":"    end"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_a2ec0490","line":22,"in_reply_to":"1a26ad4f_252a437f","updated":"2015-10-30 01:22:32.000000000","message":"You\u0027re well defending your ground, I wouldn\u0027t expect less from you! ;)\n\nI understand 1. and 2.\nThat said do you agree with the fact that set? and parameter_set? do not manipulate any instance data from the caller object?\nIf yes then they could be used as class methods, that\u0027s my point. \nIn which case they can go into 2. whether you want or not to use inheritance.","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"}],"lib/puppet_x/keystone/type.rb":[{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"8e3fe6f15591caa3a72c5d37a830cd5bc3957a17","unresolved":false,"context_lines":[{"line_number":5,"context_line":""},{"line_number":6,"context_line":"module PuppetX"},{"line_number":7,"context_line":"  module Keystone"},{"line_number":8,"context_line":"    module Type"},{"line_number":9,"context_line":"      def self.domain_defaultto(resource)"},{"line_number":10,"context_line":"        current_default \u003d resource.provider.class.class_variable_get(\u0027@@default_domain\u0027)"},{"line_number":11,"context_line":"        if current_default.nil?"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_b3943de2","line":8,"updated":"2015-10-30 17:06:00.000000000","message":"Hum, I don\u0027t talk about a line between type and provider but between keystone specific (which this file is) and composite namevar specific.  This code do not belong to composite namevar it\u0027s only a place for common keystone type code.","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"a17e08e52999f031b3ad37a70a99857f572f65ee","unresolved":false,"context_lines":[{"line_number":5,"context_line":""},{"line_number":6,"context_line":"module PuppetX"},{"line_number":7,"context_line":"  module Keystone"},{"line_number":8,"context_line":"    module Type"},{"line_number":9,"context_line":"      def self.domain_defaultto(resource)"},{"line_number":10,"context_line":"        current_default \u003d resource.provider.class.class_variable_get(\u0027@@default_domain\u0027)"},{"line_number":11,"context_line":"        if current_default.nil?"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_052707b7","line":8,"updated":"2015-10-29 20:02:09.000000000","message":"It was to pave the way for a clean split between composite namevar code and keystone code.  The helpers.rb should be with composite.  Their enable the user to have two mutually exclusive namevar like in user_role with domain and project are optional.  They provide utilities to check it for both the provider and the type.\nType.rb here is really keystone dependant.  I think that in the spirit of separation of the two (composite namevar and keystone) it should be let as it is.","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"93934db269bc4119519d4c82acaad6b95472b744","unresolved":false,"context_lines":[{"line_number":5,"context_line":""},{"line_number":6,"context_line":"module PuppetX"},{"line_number":7,"context_line":"  module Keystone"},{"line_number":8,"context_line":"    module Type"},{"line_number":9,"context_line":"      def self.domain_defaultto(resource)"},{"line_number":10,"context_line":"        current_default \u003d resource.provider.class.class_variable_get(\u0027@@default_domain\u0027)"},{"line_number":11,"context_line":"        if current_default.nil?"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_de0fb9df","line":8,"updated":"2015-10-29 03:08:08.000000000","message":"The approach is noble but I think going too far.\n?o we really have to have a separate library for this.\nIt seems there is no clear demarcation line between 3rd party libs we use the type/provider files\n\nFor instance, lib/puppet_x/keystone/composite_namevar/helpers is used in both type and providers code.","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"d3806205a0afc256d4c52532176ecdfd47a770d0","unresolved":false,"context_lines":[{"line_number":5,"context_line":""},{"line_number":6,"context_line":"module PuppetX"},{"line_number":7,"context_line":"  module Keystone"},{"line_number":8,"context_line":"    module Type"},{"line_number":9,"context_line":"      def self.domain_defaultto(resource)"},{"line_number":10,"context_line":"        current_default \u003d resource.provider.class.class_variable_get(\u0027@@default_domain\u0027)"},{"line_number":11,"context_line":"        if current_default.nil?"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_0208582b","line":8,"in_reply_to":"1a26ad4f_052707b7","updated":"2015-10-30 01:22:32.000000000","message":"Ok I understand the idea.\nBut I still don\u0027t really see any difference because we\u0027re talking about a clear line between type and provider inside the composite namevar namespace, the latter already is separating things from keystone code anyway.","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"5727c631633ab3af8136fdd82d279f5388ecacea","unresolved":false,"context_lines":[{"line_number":4,"context_line":"module PuppetX"},{"line_number":5,"context_line":"  module Keystone"},{"line_number":6,"context_line":"    module Type"},{"line_number":7,"context_line":"      def self.domain_defaultto(resource)"},{"line_number":8,"context_line":"        current_default \u003d resource.provider.class.class_variable_get(\u0027@@default_domain\u0027)"},{"line_number":9,"context_line":"        if current_default.nil?"},{"line_number":10,"context_line":"          # resource.catalog is nil when using the \"puppet resource\" command."}],"source_content_type":"text/x-ruby","patch_set":42,"id":"1a26ad4f_f37875c6","line":7,"updated":"2015-10-30 17:08:32.000000000","message":"As it\u0027s only used inside type\u0027s defaultto method, I though that it would be a good name.  Open to suggestion here.","commit_id":"c1e75919063f23991757f93255d1ea80b3587007"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"02ce99c70e427040f3ce001c637a24c71d3e875a","unresolved":false,"context_lines":[{"line_number":4,"context_line":"module PuppetX"},{"line_number":5,"context_line":"  module Keystone"},{"line_number":6,"context_line":"    module Type"},{"line_number":7,"context_line":"      def self.domain_defaultto(resource)"},{"line_number":8,"context_line":"        current_default \u003d resource.provider.class.class_variable_get(\u0027@@default_domain\u0027)"},{"line_number":9,"context_line":"        if current_default.nil?"},{"line_number":10,"context_line":"          # resource.catalog is nil when using the \"puppet resource\" command."}],"source_content_type":"text/x-ruby","patch_set":42,"id":"1a26ad4f_a2808447","line":7,"updated":"2015-10-30 02:29:27.000000000","message":"I think this name is confusing with type\u0027s method \u0027defaultto\u0027.","commit_id":"c1e75919063f23991757f93255d1ea80b3587007"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"5727c631633ab3af8136fdd82d279f5388ecacea","unresolved":false,"context_lines":[{"line_number":7,"context_line":"      def self.domain_defaultto(resource)"},{"line_number":8,"context_line":"        current_default \u003d resource.provider.class.class_variable_get(\u0027@@default_domain\u0027)"},{"line_number":9,"context_line":"        if current_default.nil?"},{"line_number":10,"context_line":"          # resource.catalog is nil when using the \"puppet resource\" command."},{"line_number":11,"context_line":"          unless resource.catalog.nil?"},{"line_number":12,"context_line":"            default_res_in_cat \u003d resource.catalog.resources.find do |r|"},{"line_number":13,"context_line":"              r.class.to_s \u003d\u003d \u0027Puppet::Type::Keystone_domain\u0027 \u0026\u0026"}],"source_content_type":"text/x-ruby","patch_set":42,"id":"1a26ad4f_137411cd","line":10,"updated":"2015-10-30 17:08:32.000000000","message":"Good catch.","commit_id":"c1e75919063f23991757f93255d1ea80b3587007"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"02ce99c70e427040f3ce001c637a24c71d3e875a","unresolved":false,"context_lines":[{"line_number":7,"context_line":"      def self.domain_defaultto(resource)"},{"line_number":8,"context_line":"        current_default \u003d resource.provider.class.class_variable_get(\u0027@@default_domain\u0027)"},{"line_number":9,"context_line":"        if current_default.nil?"},{"line_number":10,"context_line":"          # resource.catalog is nil when using the \"puppet resource\" command."},{"line_number":11,"context_line":"          unless resource.catalog.nil?"},{"line_number":12,"context_line":"            default_res_in_cat \u003d resource.catalog.resources.find do |r|"},{"line_number":13,"context_line":"              r.class.to_s \u003d\u003d \u0027Puppet::Type::Keystone_domain\u0027 \u0026\u0026"}],"source_content_type":"text/x-ruby","patch_set":42,"id":"1a26ad4f_0550625c","line":10,"updated":"2015-10-30 02:29:27.000000000","message":"That comment really belong to the else block.","commit_id":"c1e75919063f23991757f93255d1ea80b3587007"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"5727c631633ab3af8136fdd82d279f5388ecacea","unresolved":false,"context_lines":[{"line_number":15,"context_line":"            end"},{"line_number":16,"context_line":"            current_default \u003d default_res_in_cat[:name] rescue \u0027Default\u0027"},{"line_number":17,"context_line":"          else"},{"line_number":18,"context_line":"            # running \"puppet resource\".  We try to get the default from"},{"line_number":19,"context_line":"            # the keystone.conf file."},{"line_number":20,"context_line":"            default_domain_from_conf \u003d Puppet::Resource.indirection"},{"line_number":21,"context_line":"              .find(\u0027Keystone_config/identity/default_domain_id\u0027)"}],"source_content_type":"text/x-ruby","patch_set":42,"id":"1a26ad4f_b382fd92","line":18,"updated":"2015-10-30 17:08:32.000000000","message":"Done","commit_id":"c1e75919063f23991757f93255d1ea80b3587007"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"02ce99c70e427040f3ce001c637a24c71d3e875a","unresolved":false,"context_lines":[{"line_number":15,"context_line":"            end"},{"line_number":16,"context_line":"            current_default \u003d default_res_in_cat[:name] rescue \u0027Default\u0027"},{"line_number":17,"context_line":"          else"},{"line_number":18,"context_line":"            # running \"puppet resource\".  We try to get the default from"},{"line_number":19,"context_line":"            # the keystone.conf file."},{"line_number":20,"context_line":"            default_domain_from_conf \u003d Puppet::Resource.indirection"},{"line_number":21,"context_line":"              .find(\u0027Keystone_config/identity/default_domain_id\u0027)"}],"source_content_type":"text/x-ruby","patch_set":42,"id":"1a26ad4f_05af0282","line":18,"updated":"2015-10-30 02:29:27.000000000","message":"This comment should be merge with previous one","commit_id":"c1e75919063f23991757f93255d1ea80b3587007"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"5727c631633ab3af8136fdd82d279f5388ecacea","unresolved":false,"context_lines":[{"line_number":17,"context_line":"          else"},{"line_number":18,"context_line":"            # running \"puppet resource\".  We try to get the default from"},{"line_number":19,"context_line":"            # the keystone.conf file."},{"line_number":20,"context_line":"            default_domain_from_conf \u003d Puppet::Resource.indirection"},{"line_number":21,"context_line":"              .find(\u0027Keystone_config/identity/default_domain_id\u0027)"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"            if default_domain_from_conf[:ensure] \u003d\u003d :absent"}],"source_content_type":"text/x-ruby","patch_set":42,"id":"1a26ad4f_d37d39b2","line":20,"updated":"2015-10-30 17:08:32.000000000","message":"I agree with the *bad* when used in manifest as it means that you resource explicitely depends on other resources being present on the system.  They are bad, because they are hard to get right.  To really check a value, you must _first_ check the catalog (if the user is modifying the value during the run), then check the resource, hoping you didn\u0027t miss any autorequire.  In the end there can be a huge performance penalty depending on where you\u0027re using it in your code.  Prefetch may be ok: you\u0027re calling self.instance of the concerned resource only one; but create may be bad: you\u0027re calling self.instance of the other resource each time you\u0027re creating a resource.\n\nHere I think it\u0027s very safe to use as it is called only when the user is calling puppet resource, ie completly outside any puppet run.  It\u0027s only a convenient way to parse the keystone.conf file.  There is no tricky dependency to handle, and the performance wise it\u0027s only parsing a file and filling a hash once.\n\nThe rule could be like this: you may not call indirection find unless outside of puppet run (ie, puppet resource like this one.  After, knowing all the problem it may raise, it may be accepted on a case by case basis.","commit_id":"c1e75919063f23991757f93255d1ea80b3587007"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"02ce99c70e427040f3ce001c637a24c71d3e875a","unresolved":false,"context_lines":[{"line_number":17,"context_line":"          else"},{"line_number":18,"context_line":"            # running \"puppet resource\".  We try to get the default from"},{"line_number":19,"context_line":"            # the keystone.conf file."},{"line_number":20,"context_line":"            default_domain_from_conf \u003d Puppet::Resource.indirection"},{"line_number":21,"context_line":"              .find(\u0027Keystone_config/identity/default_domain_id\u0027)"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"            if default_domain_from_conf[:ensure] \u003d\u003d :absent"}],"source_content_type":"text/x-ruby","patch_set":42,"id":"1a26ad4f_05caa26d","line":20,"updated":"2015-10-30 02:29:27.000000000","message":"I believe indirections are *bad*.\nBecause not officially supported and because we found that they\u0027re not efficient although in this case it might be okay.\nWhat I\u0027d like to clarify is a standard practice we need to establish to decide if we use them, not at all. And if yes how we can guarantee things are not going to wrong with them.","commit_id":"c1e75919063f23991757f93255d1ea80b3587007"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"8710ae0802ff82b285267bae37a12e55dda885aa","unresolved":false,"context_lines":[{"line_number":17,"context_line":"          else"},{"line_number":18,"context_line":"            # running \"puppet resource\".  We try to get the default from"},{"line_number":19,"context_line":"            # the keystone.conf file."},{"line_number":20,"context_line":"            default_domain_from_conf \u003d Puppet::Resource.indirection"},{"line_number":21,"context_line":"              .find(\u0027Keystone_config/identity/default_domain_id\u0027)"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"            if default_domain_from_conf[:ensure] \u003d\u003d :absent"}],"source_content_type":"text/x-ruby","patch_set":42,"id":"1a26ad4f_d6fe75ad","line":20,"in_reply_to":"1a26ad4f_d37d39b2","updated":"2015-11-02 03:59:30.000000000","message":"Thank you for the detailed explanation.\nI make sense and I understand it\u0027s okay here because that branch is only in the puppet \u0027resource\u0027 case.\n\nMeanwhile why not just use self.class.keystone_file[\u0027identity\u0027][\u0027default_domain_id\u0027] ?","commit_id":"c1e75919063f23991757f93255d1ea80b3587007"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"5727c631633ab3af8136fdd82d279f5388ecacea","unresolved":false,"context_lines":[{"line_number":27,"context_line":"                current_default \u003d resource.provider.class"},{"line_number":28,"context_line":"                  .domain_name_from_id(default_domain_from_conf[:value])"},{"line_number":29,"context_line":"              rescue"},{"line_number":30,"context_line":"                # cannot find default domain, we bail out"},{"line_number":31,"context_line":"                err(\u0027The default domain cannot be guessed from your current installation. \u0027 \\"},{"line_number":32,"context_line":"                    \u0027Please check that keystone is working properly\u0027)"},{"line_number":33,"context_line":"              end"}],"source_content_type":"text/x-ruby","patch_set":42,"id":"1a26ad4f_734b0588","line":30,"updated":"2015-10-30 17:08:32.000000000","message":"Done. (I\u0027m so found of comments :) ...)","commit_id":"c1e75919063f23991757f93255d1ea80b3587007"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"02ce99c70e427040f3ce001c637a24c71d3e875a","unresolved":false,"context_lines":[{"line_number":27,"context_line":"                current_default \u003d resource.provider.class"},{"line_number":28,"context_line":"                  .domain_name_from_id(default_domain_from_conf[:value])"},{"line_number":29,"context_line":"              rescue"},{"line_number":30,"context_line":"                # cannot find default domain, we bail out"},{"line_number":31,"context_line":"                err(\u0027The default domain cannot be guessed from your current installation. \u0027 \\"},{"line_number":32,"context_line":"                    \u0027Please check that keystone is working properly\u0027)"},{"line_number":33,"context_line":"              end"}],"source_content_type":"text/x-ruby","patch_set":42,"id":"1a26ad4f_859a3262","line":30,"updated":"2015-10-30 02:29:27.000000000","message":"I think the comment can be removed altogether.","commit_id":"c1e75919063f23991757f93255d1ea80b3587007"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"8710ae0802ff82b285267bae37a12e55dda885aa","unresolved":false,"context_lines":[{"line_number":27,"context_line":"                current_default \u003d resource.provider.class"},{"line_number":28,"context_line":"                  .domain_name_from_id(default_domain_from_conf[:value])"},{"line_number":29,"context_line":"              rescue"},{"line_number":30,"context_line":"                # cannot find default domain, we bail out"},{"line_number":31,"context_line":"                err(\u0027The default domain cannot be guessed from your current installation. \u0027 \\"},{"line_number":32,"context_line":"                    \u0027Please check that keystone is working properly\u0027)"},{"line_number":33,"context_line":"              end"}],"source_content_type":"text/x-ruby","patch_set":42,"id":"1a26ad4f_56290536","line":30,"in_reply_to":"1a26ad4f_734b0588","updated":"2015-11-02 03:59:30.000000000","message":"I\u0027m the \u0027no comment\u0027 keeper!\nLOL","commit_id":"c1e75919063f23991757f93255d1ea80b3587007"}],"lib/puppet_x/keystone/type/required.rb":[{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"93934db269bc4119519d4c82acaad6b95472b744","unresolved":false,"context_lines":[{"line_number":5,"context_line":"        def self.included(klass)"},{"line_number":6,"context_line":"          klass.class_eval do"},{"line_number":7,"context_line":"            defaultto do"},{"line_number":8,"context_line":"              fail(\"\u0027#{name}\u0027 is a required parameter of #{resource.class.to_s.split(\u0027::\u0027)[-1]}\")"},{"line_number":9,"context_line":"            end"},{"line_number":10,"context_line":"          end"},{"line_number":11,"context_line":"        end"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_bbca0f6d","line":8,"updated":"2015-10-29 03:08:08.000000000","message":"Maybe I missed something but I haven\u0027t found anywhere this module is included.","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"a17e08e52999f031b3ad37a70a99857f572f65ee","unresolved":false,"context_lines":[{"line_number":5,"context_line":"        def self.included(klass)"},{"line_number":6,"context_line":"          klass.class_eval do"},{"line_number":7,"context_line":"            defaultto do"},{"line_number":8,"context_line":"              fail(\"\u0027#{name}\u0027 is a required parameter of #{resource.class.to_s.split(\u0027::\u0027)[-1]}\")"},{"line_number":9,"context_line":"            end"},{"line_number":10,"context_line":"          end"},{"line_number":11,"context_line":"        end"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_2503230f","line":8,"updated":"2015-10-29 20:02:09.000000000","message":"No, you\u0027re right.  It will be used by keystone_service.  I backported it from there, but then realised that all the resource have default value and then forgot to remove it. I put it back into keystone_service","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"}],"spec/acceptance/basic_keystone_spec.rb":[{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"93934db269bc4119519d4c82acaad6b95472b744","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        description \u003d\u003e \u0027Tenant for the openstack services\u0027,"},{"line_number":105,"context_line":"        domain      \u003d\u003e \u0027service_domain\u0027,"},{"line_number":106,"context_line":"      }"},{"line_number":107,"context_line":"      keystone_tenant { \u0027openstackv3\u0027:"},{"line_number":108,"context_line":"        ensure      \u003d\u003e present,"},{"line_number":109,"context_line":"        enabled     \u003d\u003e true,"},{"line_number":110,"context_line":"        description \u003d\u003e \u0027admin tenant\u0027,"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_9b4073a7","line":107,"updated":"2015-10-29 03:08:08.000000000","message":"Just for the showing that both approaches work, keep the composite name here:\nkeystone_tenant { \u0027openstackv3::admin_domain\u0027:","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"8e3fe6f15591caa3a72c5d37a830cd5bc3957a17","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        description \u003d\u003e \u0027Tenant for the openstack services\u0027,"},{"line_number":105,"context_line":"        domain      \u003d\u003e \u0027service_domain\u0027,"},{"line_number":106,"context_line":"      }"},{"line_number":107,"context_line":"      keystone_tenant { \u0027openstackv3\u0027:"},{"line_number":108,"context_line":"        ensure      \u003d\u003e present,"},{"line_number":109,"context_line":"        enabled     \u003d\u003e true,"},{"line_number":110,"context_line":"        description \u003d\u003e \u0027admin tenant\u0027,"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_73bd4567","line":107,"updated":"2015-10-30 17:06:00.000000000","message":"So in the end I removed the \"unit test\" code and I follow you recommandation here.  I just let one \"unit kind of test\".  Hope it\u0027s okay like that. If not I remove it altogether as I\u0027m more confident now that the unit test are doing their job. I added the autorequire test for tenant that I missed and test for alias detection error in unit tests.  I also added more creation tests for user_role.  That should exercice all the code in there.","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"a17e08e52999f031b3ad37a70a99857f572f65ee","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        description \u003d\u003e \u0027Tenant for the openstack services\u0027,"},{"line_number":105,"context_line":"        domain      \u003d\u003e \u0027service_domain\u0027,"},{"line_number":106,"context_line":"      }"},{"line_number":107,"context_line":"      keystone_tenant { \u0027openstackv3\u0027:"},{"line_number":108,"context_line":"        ensure      \u003d\u003e present,"},{"line_number":109,"context_line":"        enabled     \u003d\u003e true,"},{"line_number":110,"context_line":"        description \u003d\u003e \u0027admin tenant\u0027,"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_c5202f9e","line":107,"updated":"2015-10-29 20:02:09.000000000","message":"This is tested later.  See my reply there.","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"a17e08e52999f031b3ad37a70a99857f572f65ee","unresolved":false,"context_lines":[{"line_number":108,"context_line":"        ensure      \u003d\u003e present,"},{"line_number":109,"context_line":"        enabled     \u003d\u003e true,"},{"line_number":110,"context_line":"        description \u003d\u003e \u0027admin tenant\u0027,"},{"line_number":111,"context_line":"        domain      \u003d\u003e \u0027admin_domain\u0027,"},{"line_number":112,"context_line":"      }"},{"line_number":113,"context_line":"      keystone_user { \u0027adminv3::admin_domain\u0027:"},{"line_number":114,"context_line":"        ensure      \u003d\u003e present,"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_e523eba5","line":111,"updated":"2015-10-29 20:02:09.000000000","message":"Idem.","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"93934db269bc4119519d4c82acaad6b95472b744","unresolved":false,"context_lines":[{"line_number":108,"context_line":"        ensure      \u003d\u003e present,"},{"line_number":109,"context_line":"        enabled     \u003d\u003e true,"},{"line_number":110,"context_line":"        description \u003d\u003e \u0027admin tenant\u0027,"},{"line_number":111,"context_line":"        domain      \u003d\u003e \u0027admin_domain\u0027,"},{"line_number":112,"context_line":"      }"},{"line_number":113,"context_line":"      keystone_user { \u0027adminv3::admin_domain\u0027:"},{"line_number":114,"context_line":"        ensure      \u003d\u003e present,"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_7b4557b1","line":111,"updated":"2015-10-29 03:08:08.000000000","message":"Same as above, just remove it here, as it was.","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"93934db269bc4119519d4c82acaad6b95472b744","unresolved":false,"context_lines":[{"line_number":246,"context_line":"    end"},{"line_number":247,"context_line":""},{"line_number":248,"context_line":"  end"},{"line_number":249,"context_line":"  "},{"line_number":250,"context_line":"  describe \u0027multiple key for tenant\u0027 do"},{"line_number":251,"context_line":"    let(:admin_credential) do"},{"line_number":252,"context_line":"      \u0027--os-username adminv3 --os-password a_big_secret \u0027 \\"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_bb21afb6","line":249,"updated":"2015-10-29 03:08:08.000000000","message":"It seems most of those are unit tests not acceptance tests.\n\nAlso if anything is added here anyway then it should be added in the two other acceptance tests of keystone_federation_identity_provider_spec.rb and keystone_wsgi_apache_spec.rb. Although the repetition is another story!","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"a17e08e52999f031b3ad37a70a99857f572f65ee","unresolved":false,"context_lines":[{"line_number":246,"context_line":"    end"},{"line_number":247,"context_line":""},{"line_number":248,"context_line":"  end"},{"line_number":249,"context_line":"  "},{"line_number":250,"context_line":"  describe \u0027multiple key for tenant\u0027 do"},{"line_number":251,"context_line":"    let(:admin_credential) do"},{"line_number":252,"context_line":"      \u0027--os-username adminv3 --os-password a_big_secret \u0027 \\"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_853b17d1","line":249,"updated":"2015-10-29 20:02:09.000000000","message":"Well, I have added here all the corner cases I bumped into while doing the development.  This are not unit test really, as they depend on the whole keystone installation to succeed.\n\nThe repetion is not required, I think, as it the fact that keystone is behind a http server won\u0027t change anything to the code here.  I put it in the beginning, but remove it not so much for repetition but for the time the overall acceptance test take. (but I think that the repetion is bad, bad, bad, and should be work around).  So unless you\u0027re strongminded on this one or if other people think I\u0027m wrong, I think it should be let as it is.","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"8e3fe6f15591caa3a72c5d37a830cd5bc3957a17","unresolved":false,"context_lines":[{"line_number":246,"context_line":"    end"},{"line_number":247,"context_line":""},{"line_number":248,"context_line":"  end"},{"line_number":249,"context_line":"  "},{"line_number":250,"context_line":"  describe \u0027multiple key for tenant\u0027 do"},{"line_number":251,"context_line":"    let(:admin_credential) do"},{"line_number":252,"context_line":"      \u0027--os-username adminv3 --os-password a_big_secret \u0027 \\"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_93c021e2","line":249,"updated":"2015-10-30 17:06:00.000000000","message":"Well, my rpsec knowledge of puppet has increased those last time and I\u0027m now able to reach the prefetch method, which was something I was lacking before, and made those test very useful.  But, even the unit test seems robust, I still rather have a run of the real thing, only to discard any doubts into the mind of those less rspec-puppet savvy.  Furthermore it provide a clear exemple of something working and lastly the rspec-puppet code may make assumption which aren\u0027t verified all the time.\n\nThat said, I see your point, and what you mean by \"they look like unit test\", as I put it I did use them when rpsec tests weren\u0027t reaching all part of the code.  Now that I have them, they may not be that useful anymore.\n\nI will have a second look into the rspec and try to find if something is covered here that is not covered there.  Maybe I add some check into the current run of the keystone acceptance and ajust name to use composite namevar and remove the code here.  \u003d\u003d\u003e that eventually what I did.\n\nThank for the insight.","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"d3806205a0afc256d4c52532176ecdfd47a770d0","unresolved":false,"context_lines":[{"line_number":246,"context_line":"    end"},{"line_number":247,"context_line":""},{"line_number":248,"context_line":"  end"},{"line_number":249,"context_line":"  "},{"line_number":250,"context_line":"  describe \u0027multiple key for tenant\u0027 do"},{"line_number":251,"context_line":"    let(:admin_credential) do"},{"line_number":252,"context_line":"      \u0027--os-username adminv3 --os-password a_big_secret \u0027 \\"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_45c65a9b","line":249,"in_reply_to":"1a26ad4f_853b17d1","updated":"2015-10-30 01:22:32.000000000","message":"\u003e Well, I have added here all the corner cases I bumped into while doing the development. This are not unit test really, as they depend on the whole keystone installation to succeed.\n\nIs that because of default domain only?\nOtherwise I don\u0027t see why we couldn\u0027t move them (or some) to the unit tests?\n\n\u003e ... but I think that the repetion is bad, bad, bad, and should be work around\n+1","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"5dbb9da4161aa5bc40d4b3d3d84a8035a3a93666","unresolved":false,"context_lines":[{"line_number":270,"context_line":"      it \u0027should not do any modification\u0027 do"},{"line_number":271,"context_line":"        apply_manifest(pp, :catch_changes \u003d\u003e true)"},{"line_number":272,"context_line":"      end"},{"line_number":273,"context_line":"    end"},{"line_number":274,"context_line":"  end"},{"line_number":275,"context_line":"end"}],"source_content_type":"text/x-ruby","patch_set":43,"id":"1a26ad4f_f697f145","line":273,"updated":"2015-11-02 05:39:29.000000000","message":"Well done, now that\u0027s crystal clear!","commit_id":"fa2638354ad9f6b7b27b6a0f539eceeb9bfe8210"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"63172159f19ea950d5167a5827e69db82c90c49a","unresolved":false,"context_lines":[{"line_number":270,"context_line":"      it \u0027should not do any modification\u0027 do"},{"line_number":271,"context_line":"        apply_manifest(pp, :catch_changes \u003d\u003e true)"},{"line_number":272,"context_line":"      end"},{"line_number":273,"context_line":"    end"},{"line_number":274,"context_line":"  end"},{"line_number":275,"context_line":"end"}],"source_content_type":"text/x-ruby","patch_set":43,"id":"1a26ad4f_a4092786","line":273,"in_reply_to":"1a26ad4f_f697f145","updated":"2015-11-02 19:00:13.000000000","message":"Thanks.","commit_id":"fa2638354ad9f6b7b27b6a0f539eceeb9bfe8210"}],"spec/unit/provider/keystone_tenant/openstack_spec.rb":[{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"93934db269bc4119519d4c82acaad6b95472b744","unresolved":false,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"  let(:resource_attrs) do"},{"line_number":16,"context_line":"    {"},{"line_number":17,"context_line":"      :name         \u003d\u003e \u0027foo\u0027,"},{"line_number":18,"context_line":"      :description  \u003d\u003e \u0027foo\u0027,"},{"line_number":19,"context_line":"      :ensure       \u003d\u003e \u0027present\u0027,"},{"line_number":20,"context_line":"      :enabled      \u003d\u003e \u0027True\u0027,"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_fe0cf53c","line":17,"updated":"2015-10-29 03:08:08.000000000","message":"I know it\u0027s legacy code but can we please take the opportunity to use something more meaningful than foo everywhere? It makes the test super hard to read (it drives me \u0027mad\u0027 :).","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"a17e08e52999f031b3ad37a70a99857f572f65ee","unresolved":false,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"  let(:resource_attrs) do"},{"line_number":16,"context_line":"    {"},{"line_number":17,"context_line":"      :name         \u003d\u003e \u0027foo\u0027,"},{"line_number":18,"context_line":"      :description  \u003d\u003e \u0027foo\u0027,"},{"line_number":19,"context_line":"      :ensure       \u003d\u003e \u0027present\u0027,"},{"line_number":20,"context_line":"      :enabled      \u003d\u003e \u0027True\u0027,"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_c5090f2e","line":17,"updated":"2015-10-29 20:02:09.000000000","message":"foo, bar have disapeared in their own chaos!","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"d3806205a0afc256d4c52532176ecdfd47a770d0","unresolved":false,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"  let(:resource_attrs) do"},{"line_number":16,"context_line":"    {"},{"line_number":17,"context_line":"      :name         \u003d\u003e \u0027foo\u0027,"},{"line_number":18,"context_line":"      :description  \u003d\u003e \u0027foo\u0027,"},{"line_number":19,"context_line":"      :ensure       \u003d\u003e \u0027present\u0027,"},{"line_number":20,"context_line":"      :enabled      \u003d\u003e \u0027True\u0027,"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_25523ede","line":17,"in_reply_to":"1a26ad4f_c5090f2e","updated":"2015-10-30 01:22:32.000000000","message":"Super!\nThanks","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"a17e08e52999f031b3ad37a70a99857f572f65ee","unresolved":false,"context_lines":[{"line_number":125,"context_line":"        provider.class.expects(:openstack)"},{"line_number":126,"context_line":"          .with(\u0027project\u0027, \u0027list\u0027, \u0027--quiet\u0027, \u0027--format\u0027, \u0027csv\u0027, \u0027--long\u0027)"},{"line_number":127,"context_line":"          .returns(\u0027\"ID\",\"Name\",\"Domain ID\",\"Description\",\"Enabled\""},{"line_number":128,"context_line":"\"1cb05cfed7c24279be884ba4f6520262\",\"name\",\"default\",\"A project\",True"},{"line_number":129,"context_line":"\"2cb05cfed7c24279be884ba4f6520262\",\"foo\",\"bar_domain_id\",\"A bar\",True"},{"line_number":130,"context_line":"\u0027"},{"line_number":131,"context_line":"                  )"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_e50c4b3d","line":128,"updated":"2015-10-29 20:02:09.000000000","message":"Done","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"93934db269bc4119519d4c82acaad6b95472b744","unresolved":false,"context_lines":[{"line_number":125,"context_line":"        provider.class.expects(:openstack)"},{"line_number":126,"context_line":"          .with(\u0027project\u0027, \u0027list\u0027, \u0027--quiet\u0027, \u0027--format\u0027, \u0027csv\u0027, \u0027--long\u0027)"},{"line_number":127,"context_line":"          .returns(\u0027\"ID\",\"Name\",\"Domain ID\",\"Description\",\"Enabled\""},{"line_number":128,"context_line":"\"1cb05cfed7c24279be884ba4f6520262\",\"name\",\"default\",\"A project\",True"},{"line_number":129,"context_line":"\"2cb05cfed7c24279be884ba4f6520262\",\"foo\",\"bar_domain_id\",\"A bar\",True"},{"line_number":130,"context_line":"\u0027"},{"line_number":131,"context_line":"                  )"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_de6ed9c8","line":128,"updated":"2015-10-29 03:08:08.000000000","message":"Same sort of thing here, a more readable id (project1_id or equivalent), makes the rspec easier to maintain. Again I understand it\u0027s was there and it means changing them everywhere in the file to be consistent.","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"a17e08e52999f031b3ad37a70a99857f572f65ee","unresolved":false,"context_lines":[{"line_number":126,"context_line":"          .with(\u0027project\u0027, \u0027list\u0027, \u0027--quiet\u0027, \u0027--format\u0027, \u0027csv\u0027, \u0027--long\u0027)"},{"line_number":127,"context_line":"          .returns(\u0027\"ID\",\"Name\",\"Domain ID\",\"Description\",\"Enabled\""},{"line_number":128,"context_line":"\"1cb05cfed7c24279be884ba4f6520262\",\"name\",\"default\",\"A project\",True"},{"line_number":129,"context_line":"\"2cb05cfed7c24279be884ba4f6520262\",\"foo\",\"bar_domain_id\",\"A bar\",True"},{"line_number":130,"context_line":"\u0027"},{"line_number":131,"context_line":"                  )"},{"line_number":132,"context_line":"      end"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_85147744","line":129,"updated":"2015-10-29 20:02:09.000000000","message":"Done","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"93934db269bc4119519d4c82acaad6b95472b744","unresolved":false,"context_lines":[{"line_number":126,"context_line":"          .with(\u0027project\u0027, \u0027list\u0027, \u0027--quiet\u0027, \u0027--format\u0027, \u0027csv\u0027, \u0027--long\u0027)"},{"line_number":127,"context_line":"          .returns(\u0027\"ID\",\"Name\",\"Domain ID\",\"Description\",\"Enabled\""},{"line_number":128,"context_line":"\"1cb05cfed7c24279be884ba4f6520262\",\"name\",\"default\",\"A project\",True"},{"line_number":129,"context_line":"\"2cb05cfed7c24279be884ba4f6520262\",\"foo\",\"bar_domain_id\",\"A bar\",True"},{"line_number":130,"context_line":"\u0027"},{"line_number":131,"context_line":"                  )"},{"line_number":132,"context_line":"      end"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_fe7155a9","line":129,"updated":"2015-10-29 03:08:08.000000000","message":"Same as above.","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"a17e08e52999f031b3ad37a70a99857f572f65ee","unresolved":false,"context_lines":[{"line_number":128,"context_line":"\"1cb05cfed7c24279be884ba4f6520262\",\"name\",\"default\",\"A project\",True"},{"line_number":129,"context_line":"\"2cb05cfed7c24279be884ba4f6520262\",\"foo\",\"bar_domain_id\",\"A bar\",True"},{"line_number":130,"context_line":"\u0027"},{"line_number":131,"context_line":"                  )"},{"line_number":132,"context_line":"      end"},{"line_number":133,"context_line":"      let(:resources) do"},{"line_number":134,"context_line":"        [Puppet::Type.type(:keystone_tenant).new(:title \u003d\u003e \u0027name\u0027, :ensure \u003d\u003e :absent),"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_a50f332d","line":131,"updated":"2015-10-29 20:02:09.000000000","message":"In some way it\u0027s perfectly aligned with the opening parethesis ... but I\u0027ll move it at the beginning of the block.  Hope it\u0027s ok.","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"93934db269bc4119519d4c82acaad6b95472b744","unresolved":false,"context_lines":[{"line_number":128,"context_line":"\"1cb05cfed7c24279be884ba4f6520262\",\"name\",\"default\",\"A project\",True"},{"line_number":129,"context_line":"\"2cb05cfed7c24279be884ba4f6520262\",\"foo\",\"bar_domain_id\",\"A bar\",True"},{"line_number":130,"context_line":"\u0027"},{"line_number":131,"context_line":"                  )"},{"line_number":132,"context_line":"      end"},{"line_number":133,"context_line":"      let(:resources) do"},{"line_number":134,"context_line":"        [Puppet::Type.type(:keystone_tenant).new(:title \u003d\u003e \u0027name\u0027, :ensure \u003d\u003e :absent),"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_5b073b1c","line":131,"updated":"2015-10-29 03:08:08.000000000","message":"Please remove this trailing space/padding","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"02ce99c70e427040f3ce001c637a24c71d3e875a","unresolved":false,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"  let(:resource_attrs) do"},{"line_number":16,"context_line":"    {"},{"line_number":17,"context_line":"      :name         \u003d\u003e \u0027project_one\u0027,"},{"line_number":18,"context_line":"      :description  \u003d\u003e \u0027Project One\u0027,"},{"line_number":19,"context_line":"      :ensure       \u003d\u003e \u0027present\u0027,"},{"line_number":20,"context_line":"      :enabled      \u003d\u003e \u0027True\u0027"}],"source_content_type":"text/x-ruby","patch_set":42,"id":"1a26ad4f_d3ea300f","line":17,"updated":"2015-10-30 02:29:27.000000000","message":"Feels good!","commit_id":"c1e75919063f23991757f93255d1ea80b3587007"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"5727c631633ab3af8136fdd82d279f5388ecacea","unresolved":false,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"  let(:resource_attrs) do"},{"line_number":16,"context_line":"    {"},{"line_number":17,"context_line":"      :name         \u003d\u003e \u0027project_one\u0027,"},{"line_number":18,"context_line":"      :description  \u003d\u003e \u0027Project One\u0027,"},{"line_number":19,"context_line":"      :ensure       \u003d\u003e \u0027present\u0027,"},{"line_number":20,"context_line":"      :enabled      \u003d\u003e \u0027True\u0027"}],"source_content_type":"text/x-ruby","patch_set":42,"id":"1a26ad4f_33458d6f","line":17,"updated":"2015-10-30 17:08:32.000000000","message":"My pleasure.","commit_id":"c1e75919063f23991757f93255d1ea80b3587007"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"5727c631633ab3af8136fdd82d279f5388ecacea","unresolved":false,"context_lines":[{"line_number":129,"context_line":"\"1cb05cfed7c24279be884ba4f6520262\",\"project_one\",\"default\",\"A project\",True"},{"line_number":130,"context_line":"\"2cb05cfed7c24279be884ba4f6520262\",\"project_one\",\"domain_two_id\",\"A domain_two\",True"},{"line_number":131,"context_line":"\u0027"},{"line_number":132,"context_line":"                  )"},{"line_number":133,"context_line":"      end"},{"line_number":134,"context_line":"      let(:resources) do"},{"line_number":135,"context_line":"        [Puppet::Type.type(:keystone_tenant).new(:title \u003d\u003e \u0027project_one\u0027, :ensure \u003d\u003e :absent),"}],"source_content_type":"text/x-ruby","patch_set":42,"id":"1a26ad4f_53484987","line":132,"updated":"2015-10-30 17:08:32.000000000","message":"That the first choice of my emacs ruby mode.  I have to hit \"TAB\" to put it back, which I forget some time.  And when I\u0027m reindenting a block, it goes directly there, and I have to hit \"TAB\" again.  I have to dig into the doc to see how I get it the other way round.  Sorry for the inconvenience...\nDone","commit_id":"c1e75919063f23991757f93255d1ea80b3587007"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"02ce99c70e427040f3ce001c637a24c71d3e875a","unresolved":false,"context_lines":[{"line_number":129,"context_line":"\"1cb05cfed7c24279be884ba4f6520262\",\"project_one\",\"default\",\"A project\",True"},{"line_number":130,"context_line":"\"2cb05cfed7c24279be884ba4f6520262\",\"project_one\",\"domain_two_id\",\"A domain_two\",True"},{"line_number":131,"context_line":"\u0027"},{"line_number":132,"context_line":"                  )"},{"line_number":133,"context_line":"      end"},{"line_number":134,"context_line":"      let(:resources) do"},{"line_number":135,"context_line":"        [Puppet::Type.type(:keystone_tenant).new(:title \u003d\u003e \u0027project_one\u0027, :ensure \u003d\u003e :absent),"}],"source_content_type":"text/x-ruby","patch_set":42,"id":"1a26ad4f_73ac7ced","line":132,"updated":"2015-10-30 02:29:27.000000000","message":"Well we just don\u0027t do that anywhere else.\nSo to be consistent let\u0027s not do it.\n\nBut I also believe, that we use 2 spaces for indentation, and to be consistent in that regard. For example:\n\nprovider.class.expects(:openstack)\n  .with(\u0027project\u0027, \u0027list\u0027, \u0027--quiet\u0027, \u0027--format\u0027, \u0027csv\u0027, \u0027--long\u0027)\n\nand not align with previous line.\nprovider.class.expects(:openstack)\n              .with(\u0027project\u0027, \u0027list\u0027, \u0027--quiet\u0027, \u0027--format\u0027, \u0027csv\u0027, \u0027--long\u0027)\n\nYou actually fixed the later in previous reviews and I\u0027m glad you started it.","commit_id":"c1e75919063f23991757f93255d1ea80b3587007"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"8710ae0802ff82b285267bae37a12e55dda885aa","unresolved":false,"context_lines":[{"line_number":129,"context_line":"\"1cb05cfed7c24279be884ba4f6520262\",\"project_one\",\"default\",\"A project\",True"},{"line_number":130,"context_line":"\"2cb05cfed7c24279be884ba4f6520262\",\"project_one\",\"domain_two_id\",\"A domain_two\",True"},{"line_number":131,"context_line":"\u0027"},{"line_number":132,"context_line":"                  )"},{"line_number":133,"context_line":"      end"},{"line_number":134,"context_line":"      let(:resources) do"},{"line_number":135,"context_line":"        [Puppet::Type.type(:keystone_tenant).new(:title \u003d\u003e \u0027project_one\u0027, :ensure \u003d\u003e :absent),"}],"source_content_type":"text/x-ruby","patch_set":42,"id":"1a26ad4f_f6e931e7","line":132,"in_reply_to":"1a26ad4f_53484987","updated":"2015-11-02 03:59:30.000000000","message":"Thanks.\n\nBTW, after years of using Emacs I finally traded it for Atom, there is absolutely no looking back! You\u0027ve got to watch the video on this page https://atom.io/","commit_id":"c1e75919063f23991757f93255d1ea80b3587007"}],"spec/unit/provider/keystone_user_role/openstack_spec.rb":[{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"5dbb9da4161aa5bc40d4b3d3d84a8035a3a93666","unresolved":false,"context_lines":[{"line_number":46,"context_line":"\"role1_id\",\"role1\",\"project1\",\"user1\""},{"line_number":47,"context_line":"\"role2_id\",\"role2\",\"project1\",\"user1\""},{"line_number":48,"context_line":"\u0027"},{"line_number":49,"context_line":"        )"},{"line_number":50,"context_line":"        provider_class.expects(:openstack)"},{"line_number":51,"context_line":"          .with(\u0027role\u0027, \u0027add\u0027,"},{"line_number":52,"context_line":"          [\u0027role1\u0027, \u0027--project\u0027, \u0027project1_id\u0027, \u0027--user\u0027, \u0027user1_id\u0027])"}],"source_content_type":"text/x-ruby","patch_set":43,"id":"1a26ad4f_96bd5db9","line":49,"updated":"2015-11-02 05:39:29.000000000","message":"The empty spaces.","commit_id":"fa2638354ad9f6b7b27b6a0f539eceeb9bfe8210"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"c6c31f1eaf0d65d36c57bcab3f019465b65bcad4","unresolved":false,"context_lines":[{"line_number":46,"context_line":"\"role1_id\",\"role1\",\"project1\",\"user1\""},{"line_number":47,"context_line":"\"role2_id\",\"role2\",\"project1\",\"user1\""},{"line_number":48,"context_line":"\u0027"},{"line_number":49,"context_line":"        )"},{"line_number":50,"context_line":"        provider_class.expects(:openstack)"},{"line_number":51,"context_line":"          .with(\u0027role\u0027, \u0027add\u0027,"},{"line_number":52,"context_line":"          [\u0027role1\u0027, \u0027--project\u0027, \u0027project1_id\u0027, \u0027--user\u0027, \u0027user1_id\u0027])"}],"source_content_type":"text/x-ruby","patch_set":43,"id":"1a26ad4f_6219bad6","line":49,"in_reply_to":"1a26ad4f_6428efe2","updated":"2015-11-02 23:45:26.000000000","message":"According to  https://github.com/bbatsov/ruby-style-guide\nboth approaches are acceptable\nBut the latter reference mentions it matters to be consistent\n\nTherefore to use that indentation style means\n1. The opening bracket should be like this:\n\n.with(\n  \u0027role\u0027, \u0027list\u0027, ....\n)\n\nActually following the standard it should be more like this:\n.with(\n  \u0027role\u0027,\n  \u0027list\u0027,\n  ...\n)\n\n2. Consistency would mean it applies everywhere, at least on the same file\n\nFinally I would rather stick with the current practice because the .returns is a special case, the goal is readability and not taking unnecesary space, don\u0027t we?","commit_id":"fa2638354ad9f6b7b27b6a0f539eceeb9bfe8210"},{"author":{"_account_id":6926,"name":"Bogdan Dobrelya","email":"bdobreli@redhat.com","username":"bogdando"},"change_message_id":"c19f743130c57849cdf65dc7e52d52a2a51a3bbe","unresolved":false,"context_lines":[{"line_number":46,"context_line":"\"role1_id\",\"role1\",\"project1\",\"user1\""},{"line_number":47,"context_line":"\"role2_id\",\"role2\",\"project1\",\"user1\""},{"line_number":48,"context_line":"\u0027"},{"line_number":49,"context_line":"        )"},{"line_number":50,"context_line":"        provider_class.expects(:openstack)"},{"line_number":51,"context_line":"          .with(\u0027role\u0027, \u0027add\u0027,"},{"line_number":52,"context_line":"          [\u0027role1\u0027, \u0027--project\u0027, \u0027project1_id\u0027, \u0027--user\u0027, \u0027user1_id\u0027])"}],"source_content_type":"text/x-ruby","patch_set":43,"id":"1a26ad4f_e9b94f16","line":49,"in_reply_to":"1a26ad4f_96bd5db9","updated":"2015-11-02 08:24:01.000000000","message":"This looks like a correct indentation closing the provider stanza","commit_id":"fa2638354ad9f6b7b27b6a0f539eceeb9bfe8210"},{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"63172159f19ea950d5167a5827e69db82c90c49a","unresolved":false,"context_lines":[{"line_number":46,"context_line":"\"role1_id\",\"role1\",\"project1\",\"user1\""},{"line_number":47,"context_line":"\"role2_id\",\"role2\",\"project1\",\"user1\""},{"line_number":48,"context_line":"\u0027"},{"line_number":49,"context_line":"        )"},{"line_number":50,"context_line":"        provider_class.expects(:openstack)"},{"line_number":51,"context_line":"          .with(\u0027role\u0027, \u0027add\u0027,"},{"line_number":52,"context_line":"          [\u0027role1\u0027, \u0027--project\u0027, \u0027project1_id\u0027, \u0027--user\u0027, \u0027user1_id\u0027])"}],"source_content_type":"text/x-ruby","patch_set":43,"id":"1a26ad4f_6428efe2","line":49,"in_reply_to":"1a26ad4f_e9b94f16","updated":"2015-11-02 19:00:13.000000000","message":"I tend to agree with Bogdan.","commit_id":"fa2638354ad9f6b7b27b6a0f539eceeb9bfe8210"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"5dbb9da4161aa5bc40d4b3d3d84a8035a3a93666","unresolved":false,"context_lines":[{"line_number":59,"context_line":"          .returns(\u0027name\u003d\"project1\""},{"line_number":60,"context_line":"id\u003d\"project1_id\""},{"line_number":61,"context_line":"\u0027"},{"line_number":62,"context_line":"        )"},{"line_number":63,"context_line":"        provider_class.expects(:openstack)"},{"line_number":64,"context_line":"          .with(\u0027user\u0027, \u0027show\u0027, \u0027--format\u0027, \u0027shell\u0027,"},{"line_number":65,"context_line":"          [\u0027user1\u0027, \u0027--domain\u0027, \u0027domain1\u0027])"}],"source_content_type":"text/x-ruby","patch_set":43,"id":"1a26ad4f_36aec981","line":62,"updated":"2015-11-02 05:39:29.000000000","message":"Same as above","commit_id":"fa2638354ad9f6b7b27b6a0f539eceeb9bfe8210"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"5dbb9da4161aa5bc40d4b3d3d84a8035a3a93666","unresolved":false,"context_lines":[{"line_number":66,"context_line":"          .returns(\u0027name\u003d\"user1\""},{"line_number":67,"context_line":"id\u003d\"user1_id\""},{"line_number":68,"context_line":"\u0027"},{"line_number":69,"context_line":"        )"},{"line_number":70,"context_line":"      end"},{"line_number":71,"context_line":"      include_examples \u0027create the correct resource\u0027, ["},{"line_number":72,"context_line":"        {"}],"source_content_type":"text/x-ruby","patch_set":43,"id":"1a26ad4f_f6ccd16b","line":69,"updated":"2015-11-02 05:39:29.000000000","message":"Same as above","commit_id":"fa2638354ad9f6b7b27b6a0f539eceeb9bfe8210"}],"spec/unit/type/keystone_user_role_spec.rb":[{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"3e86546b9a83b7a550a6f5bdd34d82d0ccd83b32","unresolved":false,"context_lines":[{"line_number":92,"context_line":"  describe \u0027name::domain::foo@project\u0027 do"},{"line_number":93,"context_line":"    include_examples \u0027croak on the title\u0027"},{"line_number":94,"context_line":"  end"},{"line_number":95,"context_line":"  describe \u0027name::dom@ain@project\u0027 do"},{"line_number":96,"context_line":"    include_examples \u0027croak on the title\u0027"},{"line_number":97,"context_line":"  end"},{"line_number":98,"context_line":"  describe \u0027name::domain@\u0027 do"}],"source_content_type":"text/x-ruby","patch_set":31,"id":"1a26ad4f_c8097133","line":95,"updated":"2015-10-27 16:51:02.000000000","message":"This is already the case.  You can have an user with @ in its name.  The only thing forbiden is two consecutive colons in the username and the project\u0027s name.  The test here prevent show that it\u0027s illegal to have a @ in the user\u0027s *domain* name.  I add an exemple matching the MS AD you give to show that it works.","commit_id":"5e6f1ca189537af436efdf9f09119d9f3c160bf2"},{"author":{"_account_id":9983,"name":"Richard Megginson","email":"rmeggins@redhat.com","username":"rmeggins"},"change_message_id":"d031dae3b5d547a23449817713d91a24da704cd3","unresolved":false,"context_lines":[{"line_number":92,"context_line":"  describe \u0027name::domain::foo@project\u0027 do"},{"line_number":93,"context_line":"    include_examples \u0027croak on the title\u0027"},{"line_number":94,"context_line":"  end"},{"line_number":95,"context_line":"  describe \u0027name::dom@ain@project\u0027 do"},{"line_number":96,"context_line":"    include_examples \u0027croak on the title\u0027"},{"line_number":97,"context_line":"  end"},{"line_number":98,"context_line":"  describe \u0027name::domain@\u0027 do"}],"source_content_type":"text/x-ruby","patch_set":31,"id":"1a26ad4f_9cd8a4f1","line":95,"updated":"2015-10-27 12:25:18.000000000","message":"This should be ok.  Note that the way it works now, keystone_user_role only splits on the last \u0027@\u0027 in the name.  So something like::\n\n    keystone_user_role{ \u0027user@foo.com@project\u0027:\n\nis split into user \"user@foo.com\" and project \"project\".  For example, MS AD may use \"username@REALM\" for user names.","commit_id":"5e6f1ca189537af436efdf9f09119d9f3c160bf2"}],"spec/unit/type/keystone_user_spec.rb":[{"author":{"_account_id":8297,"name":"Sofer Athlan-Guyot","email":"sathlang@redhat.com","username":"chem"},"change_message_id":"a17e08e52999f031b3ad37a70a99857f572f65ee","unresolved":false,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    context \u0027domain autorequire from title\u0027 do"},{"line_number":28,"context_line":"      let(:user) do"},{"line_number":29,"context_line":"        Puppet::Type.type(:keystone_user).new("},{"line_number":30,"context_line":"          :title  \u003d\u003e \u0027foo::domain_user\u0027"},{"line_number":31,"context_line":"        )"},{"line_number":32,"context_line":"      end"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_05006700","line":29,"updated":"2015-10-29 20:02:09.000000000","message":"Done","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"},{"author":{"_account_id":6525,"name":"Gilles Dubreuil","email":"gilles@redhat.com","username":"q-1illes-a"},"change_message_id":"93934db269bc4119519d4c82acaad6b95472b744","unresolved":false,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    context \u0027domain autorequire from title\u0027 do"},{"line_number":28,"context_line":"      let(:user) do"},{"line_number":29,"context_line":"        Puppet::Type.type(:keystone_user).new("},{"line_number":30,"context_line":"          :title  \u003d\u003e \u0027foo::domain_user\u0027"},{"line_number":31,"context_line":"        )"},{"line_number":32,"context_line":"      end"}],"source_content_type":"text/x-ruby","patch_set":37,"id":"1a26ad4f_be92fd9d","line":29,"updated":"2015-10-29 03:08:08.000000000","message":"That fits in one line.","commit_id":"b421493f1be4c1e3402cbe82487e8c4a101bf424"}]}