)]}'
{"src/keystone.js":[{"author":{"_account_id":22670,"name":"Mitchell Smolash","email":"mitch@smolash.com","username":"msmol"},"change_message_id":"effa4ad2d3ea4767006b7e3067d9cf0ebd29d1a8","unresolved":false,"context_lines":[{"line_number":46,"context_line":"    return this"},{"line_number":47,"context_line":"      .versions()"},{"line_number":48,"context_line":"      .then((versions) \u003d\u003e {"},{"line_number":49,"context_line":"        for (let version of versions) {"},{"line_number":50,"context_line":"          if (supportedKeystoneVersions.indexOf(version.id) \u003e -1) {"},{"line_number":51,"context_line":"            return version;"},{"line_number":52,"context_line":"          }"}],"source_content_type":"text/javascript","patch_set":9,"id":"1ac06dbe_125f3642","line":49,"updated":"2016-08-25 15:27:30.000000000","message":"Would rather see `Array` methods here. i.e.\n```\nlet version \u003d versions.find((version) \u003d\u003e {\n  return supportedKeystoneVersions.includes(version.id);\n});\nif (version) { return version; } else { throw ... }\n```\nhttps://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array","commit_id":"f231326d834059bdc880bf60a188cc6df5a13b5b"},{"author":{"_account_id":9717,"name":"Michael Krotscheck","email":"krotscheck@gmail.com","username":"krotscheck"},"change_message_id":"a63d32b772d1fcb00990a2ea894a540dbe2d5435","unresolved":false,"context_lines":[{"line_number":46,"context_line":"    return this"},{"line_number":47,"context_line":"      .versions()"},{"line_number":48,"context_line":"      .then((versions) \u003d\u003e {"},{"line_number":49,"context_line":"        for (let version of versions) {"},{"line_number":50,"context_line":"          if (supportedKeystoneVersions.indexOf(version.id) \u003e -1) {"},{"line_number":51,"context_line":"            return version;"},{"line_number":52,"context_line":"          }"}],"source_content_type":"text/javascript","patch_set":9,"id":"fa7ab95a_baee49b2","line":49,"in_reply_to":"1ac06dbe_125f3642","updated":"2016-08-26 17:22:09.000000000","message":"While I agree in principle, I\u0027m not certain that discussing preferred loop/find/other construct is a valuable conversation to have. This code works, no?","commit_id":"f231326d834059bdc880bf60a188cc6df5a13b5b"},{"author":{"_account_id":22670,"name":"Mitchell Smolash","email":"mitch@smolash.com","username":"msmol"},"change_message_id":"2ab7cc1b3ea410fcb7994f6ed7d81bb047595604","unresolved":false,"context_lines":[{"line_number":46,"context_line":"    return this"},{"line_number":47,"context_line":"      .versions()"},{"line_number":48,"context_line":"      .then((versions) \u003d\u003e {"},{"line_number":49,"context_line":"        for (let version of versions) {"},{"line_number":50,"context_line":"          if (supportedKeystoneVersions.indexOf(version.id) \u003e -1) {"},{"line_number":51,"context_line":"            return version;"},{"line_number":52,"context_line":"          }"}],"source_content_type":"text/javascript","patch_set":9,"id":"fa7ab95a_e4baeb09","line":49,"in_reply_to":"fa7ab95a_baee49b2","updated":"2016-08-26 20:18:31.000000000","message":"Well, I disagree with this not being valuable conversation. I think this early code will set a precedent for the rest of the project moving forward, and what I suggested is (in my opinion) easier to read and understand. If you want to go ahead with your original implementation, I\u0027m okay with that, but maybe we should add this as a topic of discussion for the next weekly meeting.","commit_id":"f231326d834059bdc880bf60a188cc6df5a13b5b"}],"test/functional/keystoneTest.js":[{"author":{"_account_id":22670,"name":"Mitchell Smolash","email":"mitch@smolash.com","username":"msmol"},"change_message_id":"effa4ad2d3ea4767006b7e3067d9cf0ebd29d1a8","unresolved":false,"context_lines":[{"line_number":50,"context_line":"          // Quick sanity check."},{"line_number":51,"context_line":"          const apiVersion \u003d new Version(\u0027identity\u0027, version.id);"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"          for (let i \u003d 0; i \u003c supportedApiVersions.length; i++) {"},{"line_number":54,"context_line":"            let supportedVersion \u003d supportedApiVersions[i];"},{"line_number":55,"context_line":"            if (apiVersion.equals(supportedVersion)) {"},{"line_number":56,"context_line":"              done();"}],"source_content_type":"text/javascript","patch_set":9,"id":"1ac06dbe_3547bc4c","line":53,"updated":"2016-08-25 15:27:30.000000000","message":"Again, not a huge fan of iterator loops when we could (in my opinion) much more cleanly do:\n```\nlet isSupported \u003d supportedApiVersions.every((supportedVersion) \u003d\u003e {\n  return apiVersion.equals(supportedVersion);\n});\n\n// pass / fail here based on value of isSupported\n```","commit_id":"f231326d834059bdc880bf60a188cc6df5a13b5b"}],"test/unit/keystoneTest.js":[{"author":{"_account_id":8614,"name":"Corentin Ardeois","email":"corentin.ardeois@gmail.com","username":"cardeois"},"change_message_id":"beeb944504c2863e53bf4d63317a8052e9fd83e7","unresolved":false,"context_lines":[{"line_number":107,"context_line":""},{"line_number":108,"context_line":"      // Build an invalid mock object."},{"line_number":109,"context_line":"      const mockOptions \u003d JSON.parse(JSON.stringify(mockData.root()));"},{"line_number":110,"context_line":"      mockOptions.response.versions.values.shift();"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"      fetchMock.mock(mockOptions);"},{"line_number":113,"context_line":""}],"source_content_type":"text/javascript","patch_set":7,"id":"1ac06dbe_7e3963d8","line":110,"updated":"2016-08-24 16:02:44.000000000","message":"This is fine but I\u0027m just want to propose 2 alternatives to manipulating the data in the test:\n1.Define a specific data for this test?\n2. In our company we\u0027re used to create \"builders\" to construct mocked data. There are specific json data with methods (so objects) to manipulate. Example:\n  //This would give the same data as \"mockData.root()\"\n const mockOptions \u003d aVersionResponse() \n\n  // This helps overrides versions.values to have a list of one Version\n  mockOptions \u003d aVersionResponse().withVersions(aVersion().withId(\u0027v3.7\u0027)) \n\nBuilders like that are helpful because you see directly in the test what you need to mock. However it needs to be built so it has to be enough useful in tests (multiple tests use the builders)\n\nA builder class would look like this (it\u0027s ES5 sorry)\nhttp://paste.openstack.org/show/563094/","commit_id":"cc8286c840ef036793372f00274a98ae826b1561"},{"author":{"_account_id":9717,"name":"Michael Krotscheck","email":"krotscheck@gmail.com","username":"krotscheck"},"change_message_id":"c7e5655c8d863363315ebeaa9db37ff41567191f","unresolved":false,"context_lines":[{"line_number":107,"context_line":""},{"line_number":108,"context_line":"      // Build an invalid mock object."},{"line_number":109,"context_line":"      const mockOptions \u003d JSON.parse(JSON.stringify(mockData.root()));"},{"line_number":110,"context_line":"      mockOptions.response.versions.values.shift();"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"      fetchMock.mock(mockOptions);"},{"line_number":113,"context_line":""}],"source_content_type":"text/javascript","patch_set":7,"id":"1ac06dbe_d7acb58e","line":110,"in_reply_to":"1ac06dbe_7e3963d8","updated":"2016-08-24 18:23:45.000000000","message":"I\u0027m open to builders, I\u0027m worried that building out a complex test harness structure now is going to slow us down a lot.\n\nWe\u0027ll probably have to follow more of a java-style `*.build();` pattern. From fiddling around with fetchMock, it doesn\u0027t do particularly well with non-plain-javascript objects. (see \nhttps://review.openstack.org/#/c/359353/3 )","commit_id":"cc8286c840ef036793372f00274a98ae826b1561"},{"author":{"_account_id":8614,"name":"Corentin Ardeois","email":"corentin.ardeois@gmail.com","username":"cardeois"},"change_message_id":"beeb944504c2863e53bf4d63317a8052e9fd83e7","unresolved":false,"context_lines":[{"line_number":119,"context_line":"        });"},{"line_number":120,"context_line":"    });"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"    it(\"Should NOT cache its results\", (done) \u003d\u003e {"},{"line_number":123,"context_line":"      const keystone \u003d new Keystone(mockData.config);"},{"line_number":124,"context_line":"      const mockOptions \u003d mockData.root();"},{"line_number":125,"context_line":"      fetchMock.mock(mockOptions);"}],"source_content_type":"text/javascript","patch_set":7,"id":"1ac06dbe_9ee37f33","line":122,"updated":"2016-08-24 16:02:44.000000000","message":"Why shouldn\u0027t it be cached?","commit_id":"cc8286c840ef036793372f00274a98ae826b1561"},{"author":{"_account_id":9717,"name":"Michael Krotscheck","email":"krotscheck@gmail.com","username":"krotscheck"},"change_message_id":"c7e5655c8d863363315ebeaa9db37ff41567191f","unresolved":false,"context_lines":[{"line_number":119,"context_line":"        });"},{"line_number":120,"context_line":"    });"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"    it(\"Should NOT cache its results\", (done) \u003d\u003e {"},{"line_number":123,"context_line":"      const keystone \u003d new Keystone(mockData.config);"},{"line_number":124,"context_line":"      const mockOptions \u003d mockData.root();"},{"line_number":125,"context_line":"      fetchMock.mock(mockOptions);"}],"source_content_type":"text/javascript","patch_set":7,"id":"1ac06dbe_5781e5fa","line":122,"in_reply_to":"1ac06dbe_9ee37f33","updated":"2016-08-24 18:23:45.000000000","message":"Mostly because I don\u0027t see any reason why it should be. I get that version negotiation is, usually, something that only happens once. However, the only place where we need that is when determining the service endpoint (next patch), and that one is cached (see https://review.openstack.org/#/c/356764/5).","commit_id":"cc8286c840ef036793372f00274a98ae826b1561"}]}