)]}'
{"src/OpenStack/Bootstrap.php":[{"author":{"_account_id":9700,"name":"Sam Choi","email":"sam.choi@hp.com","username":"samuelchoi"},"change_message_id":"7819e27d80f1189e907988f518ec8854210a48d9","unresolved":false,"context_lines":[{"line_number":132,"context_line":"            \u0027OpenStack\\ObjectStore\\v1\\Resource\\StreamWrapperFS\u0027"},{"line_number":133,"context_line":"        );"},{"line_number":134,"context_line":"    }"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    private static function enableStreamWrapper($scheme, $class)"},{"line_number":137,"context_line":"    {"},{"line_number":138,"context_line":"        if (in_array($scheme, stream_get_wrappers())) {"}],"source_content_type":"text/x-php","patch_set":2,"id":"3ae8d1ca_92d54fbb","line":135,"updated":"2014-05-29 09:27:40.000000000","message":"Please add documentation for this function.","commit_id":"c6a59a153dc4420a4d5be8b0e686a8db387b1a37"},{"author":{"_account_id":8512,"name":"Matt Farina","email":"matt@mattfarina.com","username":"mattfarina"},"change_message_id":"53b452c4da5af548b41bb6be8040d54c07a809d7","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        );"},{"line_number":134,"context_line":"    }"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    private static function enableStreamWrapper($scheme, $class)"},{"line_number":137,"context_line":"    {"},{"line_number":138,"context_line":"        if (in_array($scheme, stream_get_wrappers())) {"},{"line_number":139,"context_line":"            stream_wrapper_unregister($scheme);"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_393e68b9","line":136,"updated":"2014-05-30 15:40:23.000000000","message":"Why is this a private method instead of a protected one? If someone were to extend Bootstrap they would have no access to it.","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"815d0ead77310fa1a8d4e1f105cfd4a2d6564a12","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        );"},{"line_number":134,"context_line":"    }"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    private static function enableStreamWrapper($scheme, $class)"},{"line_number":137,"context_line":"    {"},{"line_number":138,"context_line":"        if (in_array($scheme, stream_get_wrappers())) {"},{"line_number":139,"context_line":"            stream_wrapper_unregister($scheme);"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_a6d78322","line":136,"in_reply_to":"1ae5cdf2_393e68b9","updated":"2014-06-02 09:49:23.000000000","message":"I can\u0027t ever see a scenario where somebody would need to override this class. This is a global entry point which needs to contain certain functionality. We could add \"final\" to ensure that things stay as-is.","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"0833f80683a11e5966c4d34b98248bee30dd5e35","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        );"},{"line_number":134,"context_line":"    }"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    private static function enableStreamWrapper($scheme, $class)"},{"line_number":137,"context_line":"    {"},{"line_number":138,"context_line":"        if (in_array($scheme, stream_get_wrappers())) {"},{"line_number":139,"context_line":"            stream_wrapper_unregister($scheme);"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_c344131d","line":136,"in_reply_to":"1ae5cdf2_8ac14056","updated":"2014-06-04 19:05:12.000000000","message":"Exactly right - we *are* limiting users, but restriction isn\u0027t always a bad thing in OOP.\n\nWhen I said that this is a global entry point, I meant the entire class - not this private method. This specific class contains essential implementations that govern how end-users connect with the rest of the SDK. For that reason, it should not be inheritable IMHO - there\u0027s no conceivable use-case for allowing end-users to override this.\n\nUnless we\u0027re absolutely certain that users would need to override a base class, we shouldn\u0027t be afraid of declaring it final. That\u0027s what the keyword is for. I\u0027d rather be conservative and program to certainties instead of unnecessarily opening up base components for extension.","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":8512,"name":"Matt Farina","email":"matt@mattfarina.com","username":"mattfarina"},"change_message_id":"41fde69aa7a7152075a8f0334006ccf95c08ac16","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        );"},{"line_number":134,"context_line":"    }"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    private static function enableStreamWrapper($scheme, $class)"},{"line_number":137,"context_line":"    {"},{"line_number":138,"context_line":"        if (in_array($scheme, stream_get_wrappers())) {"},{"line_number":139,"context_line":"            stream_wrapper_unregister($scheme);"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_8ac14056","line":136,"in_reply_to":"1ae5cdf2_a6d78322","updated":"2014-06-04 17:47:32.000000000","message":"In our previous discussions we\u0027d talked about the Bootstrap class becoming an OpenStack class and top level entry point. Given that context...\n\nI can see people overriding elements in this class.\n\nThis method isn\u0027t a global point to access things. It\u0027s an internal method that other methods on the class call. If the other methods can be changed than this should be protected.\n\nThere will be cases, many of which we can\u0027t think of right now, to override this class. Unless we want to disable that ability it shouldn\u0027t be final. That would be an intentional decision rather than letting our limited imaginations decide it.\n\nI don\u0027t want to make a decision to disable what people can do in this way. We\u0027re limiting users.","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":10161,"name":"Shaunak Kashyap","email":"shaunak.kashyap@rackspace.com","username":"ycombinator"},"change_message_id":"5e388eb90c003798b533b480a7aaa7d5f4c1fb6d","unresolved":false,"context_lines":[{"line_number":129,"context_line":"            \u0027\\OpenStack\\ObjectStore\\v1\\ObjectStorage\\StreamWrapperFS\u0027"},{"line_number":130,"context_line":"        );"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"        return ($swift \u0026\u0026 $swiftfs);"},{"line_number":133,"context_line":"    }"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    /**"}],"source_content_type":"text/x-php","patch_set":11,"id":"1ae5cdf2_f06b3c1d","side":"PARENT","line":132,"updated":"2014-06-07 00:34:45.000000000","message":"The original function definition returned a boolean but the new one doesn\u0027t. Are we deliberately changing the contract here or is this an oversight?","commit_id":"bcdcba06f71256df484759cd13e7686db548c8d5"},{"author":{"_account_id":10161,"name":"Shaunak Kashyap","email":"shaunak.kashyap@rackspace.com","username":"ycombinator"},"change_message_id":"583838aa28b526faf6bebb3147201df406dceaca","unresolved":false,"context_lines":[{"line_number":129,"context_line":"            \u0027\\OpenStack\\ObjectStore\\v1\\ObjectStorage\\StreamWrapperFS\u0027"},{"line_number":130,"context_line":"        );"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"        return ($swift \u0026\u0026 $swiftfs);"},{"line_number":133,"context_line":"    }"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    /**"}],"source_content_type":"text/x-php","patch_set":11,"id":"1ae5cdf2_a2e6c6e6","side":"PARENT","line":132,"in_reply_to":"1ae5cdf2_42a9827a","updated":"2014-06-07 17:15:34.000000000","message":"Ah, as long as it wasn\u0027t being used anywhere, we are good. Done.","commit_id":"bcdcba06f71256df484759cd13e7686db548c8d5"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"bd913a5dcc12451b0adbf261d7c2b16f7172a4db","unresolved":false,"context_lines":[{"line_number":129,"context_line":"            \u0027\\OpenStack\\ObjectStore\\v1\\ObjectStorage\\StreamWrapperFS\u0027"},{"line_number":130,"context_line":"        );"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"        return ($swift \u0026\u0026 $swiftfs);"},{"line_number":133,"context_line":"    }"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    /**"}],"source_content_type":"text/x-php","patch_set":11,"id":"1ae5cdf2_42a9827a","side":"PARENT","line":132,"in_reply_to":"1ae5cdf2_f06b3c1d","updated":"2014-06-07 17:04:03.000000000","message":"The return value of this function was not being used anywhere on the SDK - so it\u0027s a BC change. Plus this method doesn\u0027t really seem to warrant one - all it\u0027s charged with is registering a stream wrapper.","commit_id":"bcdcba06f71256df484759cd13e7686db548c8d5"},{"author":{"_account_id":10161,"name":"Shaunak Kashyap","email":"shaunak.kashyap@rackspace.com","username":"ycombinator"},"change_message_id":"5e388eb90c003798b533b480a7aaa7d5f4c1fb6d","unresolved":false,"context_lines":[{"line_number":277,"context_line":"            }"},{"line_number":278,"context_line":""},{"line_number":279,"context_line":"            $klass \u003d self::config(\u0027transport\u0027);"},{"line_number":280,"context_line":"            self::$transport \u003d new $klass($options);"},{"line_number":281,"context_line":"        }"},{"line_number":282,"context_line":""},{"line_number":283,"context_line":"        return self::$transport;"}],"source_content_type":"text/x-php","patch_set":11,"id":"1ae5cdf2_b059b490","side":"PARENT","line":280,"updated":"2014-06-07 00:34:45.000000000","message":"I\u0027m assuming self::config(\u0027transport\u0027) should always return an instance of a class that implements ClientInterface. In that case, should we add some type checking here? Specifically, should the create static method be part of ClientInterface and should we, over here, check whether $class instanceOf ClientInterface?","commit_id":"bcdcba06f71256df484759cd13e7686db548c8d5"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"bd913a5dcc12451b0adbf261d7c2b16f7172a4db","unresolved":false,"context_lines":[{"line_number":277,"context_line":"            }"},{"line_number":278,"context_line":""},{"line_number":279,"context_line":"            $klass \u003d self::config(\u0027transport\u0027);"},{"line_number":280,"context_line":"            self::$transport \u003d new $klass($options);"},{"line_number":281,"context_line":"        }"},{"line_number":282,"context_line":""},{"line_number":283,"context_line":"        return self::$transport;"}],"source_content_type":"text/x-php","patch_set":11,"id":"1ae5cdf2_62b59ecf","side":"PARENT","line":280,"in_reply_to":"1ae5cdf2_b059b490","updated":"2014-06-07 17:04:03.000000000","message":"I agree about the type checking with the self::$config[\u0027transport\u0027] - but I don\u0027t want to introduce any more scope creep to this patch. This would be a great addition in another patch.","commit_id":"bcdcba06f71256df484759cd13e7686db548c8d5"}],"src/OpenStack/Common/Transport/ClientInterface.php":[{"author":{"_account_id":8512,"name":"Matt Farina","email":"matt@mattfarina.com","username":"mattfarina"},"change_message_id":"cdff1cdd05f524a398e39fcc31aec482c2cd1e4f","unresolved":false,"context_lines":[{"line_number":95,"context_line":"     * @param array $options Configuration options, such as headers"},{"line_number":96,"context_line":"     * @return ResponseInterface"},{"line_number":97,"context_line":"     */"},{"line_number":98,"context_line":"    public function delete($uri, array $options \u003d []);"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    /**"},{"line_number":101,"context_line":"     * Sets a particular configuration option, depending on how the client"}],"source_content_type":"text/x-php","patch_set":1,"id":"7adec928_9d0f232e","line":98,"updated":"2014-05-13 03:42:57.000000000","message":"I see this moves from one method where the type (e.g., POST, GET) was passed in to methods for each type of request. This was something I considered when originally crafting the interface.\n\nConsidering that we might move to json schema where the type of request would be passed in dynamically I went with a single method.\n\nWhy move to a method for each type of request?","commit_id":"2fa3167d72667bdc10d7964ce43aa29df058d9e1"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"1cbf102478bfebfd14fea6ed3767efaaab136588","unresolved":false,"context_lines":[{"line_number":95,"context_line":"     * @param array $options Configuration options, such as headers"},{"line_number":96,"context_line":"     * @return ResponseInterface"},{"line_number":97,"context_line":"     */"},{"line_number":98,"context_line":"    public function delete($uri, array $options \u003d []);"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    /**"},{"line_number":101,"context_line":"     * Sets a particular configuration option, depending on how the client"}],"source_content_type":"text/x-php","patch_set":1,"id":"7adec928_e945c5b3","line":98,"in_reply_to":"7adec928_9d0f232e","updated":"2014-05-13 11:19:32.000000000","message":"Actually, thinking about this again - I\u0027m really undecided. Using these convenience methods are really really useful IMHO. We use them throughout our current codebase because it saves us having to remember lots of generic arguments (what position is $body? Why am I even passing in a $body arg for a GET request? etc.)\n\nIf we want to use these convenience methods with confidence, they need to be part of the interface, because if someone passes in their own HTTP client that does not define them, we get a method not defined exception.","commit_id":"2fa3167d72667bdc10d7964ce43aa29df058d9e1"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"866e714f4d874b3acfcfb855343f6d293e62a4a8","unresolved":false,"context_lines":[{"line_number":95,"context_line":"     * @param array $options Configuration options, such as headers"},{"line_number":96,"context_line":"     * @return ResponseInterface"},{"line_number":97,"context_line":"     */"},{"line_number":98,"context_line":"    public function delete($uri, array $options \u003d []);"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    /**"},{"line_number":101,"context_line":"     * Sets a particular configuration option, depending on how the client"}],"source_content_type":"text/x-php","patch_set":1,"id":"7adec928_8eb0c744","line":98,"in_reply_to":"7adec928_9d0f232e","updated":"2014-05-13 11:06:40.000000000","message":"This is a good point. All of these \"verb\" methods (post, get) are only here to offer convenience, because SDKs heavily really on them for internal functionality. \n\nLooking at this again, I agree that these convenience methods are not *essential* to our API. Perhaps we could remove them from the interface, but still offer them as convenience methods in our abstract client. This means that clients which want them can easily extend AbstractClient and still implement ClientInterface.\n\nWhat do you think?","commit_id":"2fa3167d72667bdc10d7964ce43aa29df058d9e1"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"d0d9f99f5966428f830f03e642a23d905aaf4b39","unresolved":false,"context_lines":[{"line_number":95,"context_line":"     * @param array $options Configuration options, such as headers"},{"line_number":96,"context_line":"     * @return ResponseInterface"},{"line_number":97,"context_line":"     */"},{"line_number":98,"context_line":"    public function delete($uri, array $options \u003d []);"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    /**"},{"line_number":101,"context_line":"     * Sets a particular configuration option, depending on how the client"}],"source_content_type":"text/x-php","patch_set":1,"id":"7adec928_37c5388e","line":98,"in_reply_to":"7adec928_b79b1371","updated":"2014-05-14 10:51:36.000000000","message":"I agree with both points. If we *do* end up using schemas or a DSL, there\u0027s probably no need to have these convenience methods.\n\nIf we\u0027re decided about schemas, I\u0027ll more than happy remove these convenience methods from the interface. Maybe we can talk about this in our next SDK meeting.","commit_id":"2fa3167d72667bdc10d7964ce43aa29df058d9e1"},{"author":{"_account_id":8512,"name":"Matt Farina","email":"matt@mattfarina.com","username":"mattfarina"},"change_message_id":"c902a386e126b61431befd9077925289a7f51412","unresolved":false,"context_lines":[{"line_number":95,"context_line":"     * @param array $options Configuration options, such as headers"},{"line_number":96,"context_line":"     * @return ResponseInterface"},{"line_number":97,"context_line":"     */"},{"line_number":98,"context_line":"    public function delete($uri, array $options \u003d []);"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    /**"},{"line_number":101,"context_line":"     * Sets a particular configuration option, depending on how the client"}],"source_content_type":"text/x-php","patch_set":1,"id":"7adec928_b79b1371","line":98,"in_reply_to":"7adec928_e945c5b3","updated":"2014-05-13 15:28:41.000000000","message":"Imagine the case where we have json schema. The method to use in the transport layer is defined in json. To pass those in you would likely not use convenience methods. So, why have them?\n\nAn end user using the SDK would make calls via the service client. Most cases would never touch the transport layer instead using the default. For those that would change the transport layer it would be a different one passed in and they all the interactions would be against the service client.\n\nGiven this, where would the convenience methods be useful?","commit_id":"2fa3167d72667bdc10d7964ce43aa29df058d9e1"},{"author":{"_account_id":8512,"name":"Matt Farina","email":"matt@mattfarina.com","username":"mattfarina"},"change_message_id":"cdff1cdd05f524a398e39fcc31aec482c2cd1e4f","unresolved":false,"context_lines":[{"line_number":121,"context_line":"     *"},{"line_number":122,"context_line":"     * @return string"},{"line_number":123,"context_line":"     */"},{"line_number":124,"context_line":"    public function getBaseUrl();"},{"line_number":125,"context_line":"}"}],"source_content_type":"text/x-php","patch_set":1,"id":"7adec928_5d259ba9","line":124,"range":{"start_line":124,"start_character":20,"end_line":124,"end_character":30},"updated":"2014-05-13 03:42:57.000000000","message":"Why does the interface have this method? I only see it on the interface and on the adapter but not used. Why have it?","commit_id":"2fa3167d72667bdc10d7964ce43aa29df058d9e1"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"866e714f4d874b3acfcfb855343f6d293e62a4a8","unresolved":false,"context_lines":[{"line_number":121,"context_line":"     *"},{"line_number":122,"context_line":"     * @return string"},{"line_number":123,"context_line":"     */"},{"line_number":124,"context_line":"    public function getBaseUrl();"},{"line_number":125,"context_line":"}"}],"source_content_type":"text/x-php","patch_set":1,"id":"7adec928_6e81bb49","line":124,"in_reply_to":"7adec928_5d259ba9","updated":"2014-05-13 11:06:40.000000000","message":"Another good point. There are a few use-cases, though, that I think might require this method:\n\n1. When a Compute client is constructed, for example, it will have the compute region endpoint as its base URL. The `getBaseUrl` method will be needed internally to construct URLs when executing API commands.\n\n2. There is probably also a use-case for a user wanting to know what base URL (API host) they\u0027re hitting. This would be especially useful when debugging.","commit_id":"2fa3167d72667bdc10d7964ce43aa29df058d9e1"},{"author":{"_account_id":8512,"name":"Matt Farina","email":"matt@mattfarina.com","username":"mattfarina"},"change_message_id":"c902a386e126b61431befd9077925289a7f51412","unresolved":false,"context_lines":[{"line_number":121,"context_line":"     *"},{"line_number":122,"context_line":"     * @return string"},{"line_number":123,"context_line":"     */"},{"line_number":124,"context_line":"    public function getBaseUrl();"},{"line_number":125,"context_line":"}"}],"source_content_type":"text/x-php","patch_set":1,"id":"7adec928_97e8d711","line":124,"in_reply_to":"7adec928_6e81bb49","updated":"2014-05-13 15:28:41.000000000","message":"Let me throw a little wrench in this. Say we use the same transport client for all the calls. Using this client you make a call to identity, then object storage, then compute, then object storage. How does the base URL work in a flow like this?\n\nIf I\u0027m going to debug I can set the debug mode option to true and let the transport client handle debugging. If I was going to have my own debugging setup I\u0027d likely write my own transport client that wrapped an existing client with my custom debugging. Is there another way someone would want to handle debugging where grabbing the base url of the client would be important?\n\nFor general api users I don\u0027t anticipate them needing to work with the transport layer often. The service client layer is where they\u0027d need to do most of their work without needing to deal with the transport layer. The transport layer should be swappable where any implementation can be used and doesn\u0027t impact calls made on a service client. That would mean debugging a transport layer is slightly different from debugging a service client making requests.","commit_id":"2fa3167d72667bdc10d7964ce43aa29df058d9e1"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"d0d9f99f5966428f830f03e642a23d905aaf4b39","unresolved":false,"context_lines":[{"line_number":121,"context_line":"     *"},{"line_number":122,"context_line":"     * @return string"},{"line_number":123,"context_line":"     */"},{"line_number":124,"context_line":"    public function getBaseUrl();"},{"line_number":125,"context_line":"}"}],"source_content_type":"text/x-php","patch_set":1,"id":"7adec928_97160cf0","line":124,"in_reply_to":"7adec928_97e8d711","updated":"2014-05-14 10:51:36.000000000","message":"I had a slightly different idea: instead of using a global or singleton transport client that\u0027s shared between services - I wanted each service to have its own isolated instance of the transport client object. When a service is created, the transport dependency is injected (as a new object, not an existing one) and configured for that service. Typical configuration changes to the transport client could be:\n\n- the base URL changing (to that service’s API endpoint);\n- default headers being added for that service (Glance and Marconi, for example, require custom headers);\n- custom subscribers being added for that service (if it needs it)\n\nIMHO, services have no need to share a global object for their transport client because none of them have that much shared state. Sure a lot of them will have similar transport requirements, but this can be achieved through instantiating a new transport client object - not re-using an existing one.\n\nBecause services often require their own bespoke transport configurations (Marconi/Glance, etc.), I think we need to eliminate the risk of side effects on other services and not forcing them to share objects. One service should not be able to mutate the transport client of another.","commit_id":"2fa3167d72667bdc10d7964ce43aa29df058d9e1"},{"author":{"_account_id":8512,"name":"Matt Farina","email":"matt@mattfarina.com","username":"mattfarina"},"change_message_id":"53b452c4da5af548b41bb6be8040d54c07a809d7","unresolved":false,"context_lines":[{"line_number":32,"context_line":"interface ClientInterface"},{"line_number":33,"context_line":"{"},{"line_number":34,"context_line":"    /**"},{"line_number":35,"context_line":"     * Create a new Request object."},{"line_number":36,"context_line":"     *"},{"line_number":37,"context_line":"     * @param string          $method  HTTP method"},{"line_number":38,"context_line":"     * @param string|Url      $uri     URL the request will send to"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_4558a756","line":35,"updated":"2014-05-30 15:40:23.000000000","message":"Can we get this to note that it creates a request without sending it. That to send it you use send.\n\nI\u0027m thinking for the use case of someone not familiar with the setup using it.","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"815d0ead77310fa1a8d4e1f105cfd4a2d6564a12","unresolved":false,"context_lines":[{"line_number":32,"context_line":"interface ClientInterface"},{"line_number":33,"context_line":"{"},{"line_number":34,"context_line":"    /**"},{"line_number":35,"context_line":"     * Create a new Request object."},{"line_number":36,"context_line":"     *"},{"line_number":37,"context_line":"     * @param string          $method  HTTP method"},{"line_number":38,"context_line":"     * @param string|Url      $uri     URL the request will send to"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_0696774c","line":35,"in_reply_to":"1ae5cdf2_4558a756","updated":"2014-06-02 09:49:23.000000000","message":"Done","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":8512,"name":"Matt Farina","email":"matt@mattfarina.com","username":"mattfarina"},"change_message_id":"53b452c4da5af548b41bb6be8040d54c07a809d7","unresolved":false,"context_lines":[{"line_number":53,"context_line":"    /**"},{"line_number":54,"context_line":"     * Execute a GET request."},{"line_number":55,"context_line":"     *"},{"line_number":56,"context_line":"     * @param       $uri     URL the request will send to"},{"line_number":57,"context_line":"     * @param array $options Configuration options, such as headers"},{"line_number":58,"context_line":"     * @return ResponseInterface"},{"line_number":59,"context_line":"     */"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_f9d6c0c4","line":56,"updated":"2014-05-30 15:40:23.000000000","message":"What types can the $uri be? I was expecting a string but the types in here have been omitted. Is there a reason for that?","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":8512,"name":"Matt Farina","email":"matt@mattfarina.com","username":"mattfarina"},"change_message_id":"41fde69aa7a7152075a8f0334006ccf95c08ac16","unresolved":false,"context_lines":[{"line_number":53,"context_line":"    /**"},{"line_number":54,"context_line":"     * Execute a GET request."},{"line_number":55,"context_line":"     *"},{"line_number":56,"context_line":"     * @param       $uri     URL the request will send to"},{"line_number":57,"context_line":"     * @param array $options Configuration options, such as headers"},{"line_number":58,"context_line":"     * @return ResponseInterface"},{"line_number":59,"context_line":"     */"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_6adf940d","line":56,"in_reply_to":"1ae5cdf2_464a3fc6","updated":"2014-06-04 17:47:32.000000000","message":"Imagine the case where a developer using the SDK approaches the library. They see mixed as the option but don\u0027t know what types to supply. And, they don\u0027t have a pointer to let them know what types they can use.\n\nI\u0027m thinking of the DX of the docs. Especially for those seeing them in an IDE or elsewhere.","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"0833f80683a11e5966c4d34b98248bee30dd5e35","unresolved":false,"context_lines":[{"line_number":53,"context_line":"    /**"},{"line_number":54,"context_line":"     * Execute a GET request."},{"line_number":55,"context_line":"     *"},{"line_number":56,"context_line":"     * @param       $uri     URL the request will send to"},{"line_number":57,"context_line":"     * @param array $options Configuration options, such as headers"},{"line_number":58,"context_line":"     * @return ResponseInterface"},{"line_number":59,"context_line":"     */"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_c3125308","line":56,"in_reply_to":"1ae5cdf2_6adf940d","updated":"2014-06-04 19:05:12.000000000","message":"I agree with you here. I\u0027ll change this to:\n\nstring|array|\\OpenStack\\Common\\Transport\\Url\n\nthanks for clarifying","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"815d0ead77310fa1a8d4e1f105cfd4a2d6564a12","unresolved":false,"context_lines":[{"line_number":53,"context_line":"    /**"},{"line_number":54,"context_line":"     * Execute a GET request."},{"line_number":55,"context_line":"     *"},{"line_number":56,"context_line":"     * @param       $uri     URL the request will send to"},{"line_number":57,"context_line":"     * @param array $options Configuration options, such as headers"},{"line_number":58,"context_line":"     * @return ResponseInterface"},{"line_number":59,"context_line":"     */"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_464a3fc6","line":56,"in_reply_to":"1ae5cdf2_f9d6c0c4","updated":"2014-06-02 09:49:23.000000000","message":"I don\u0027t think we should specify the data type here (so use \"mixed\" instead), because the value will go straight into a new URL object. In other words, the URL class will handle the input type logic on instantiation.","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":8512,"name":"Matt Farina","email":"matt@mattfarina.com","username":"mattfarina"},"change_message_id":"53b452c4da5af548b41bb6be8040d54c07a809d7","unresolved":false,"context_lines":[{"line_number":55,"context_line":"     *"},{"line_number":56,"context_line":"     * @param       $uri     URL the request will send to"},{"line_number":57,"context_line":"     * @param array $options Configuration options, such as headers"},{"line_number":58,"context_line":"     * @return ResponseInterface"},{"line_number":59,"context_line":"     */"},{"line_number":60,"context_line":"    public function get($uri, array $options \u003d []);"},{"line_number":61,"context_line":""}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_79f9d04c","line":58,"updated":"2014-05-30 15:40:23.000000000","message":"For IDEs, can we get the full namespace for the ResponseInterface. This applies to numerous cases in here.","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"815d0ead77310fa1a8d4e1f105cfd4a2d6564a12","unresolved":false,"context_lines":[{"line_number":55,"context_line":"     *"},{"line_number":56,"context_line":"     * @param       $uri     URL the request will send to"},{"line_number":57,"context_line":"     * @param array $options Configuration options, such as headers"},{"line_number":58,"context_line":"     * @return ResponseInterface"},{"line_number":59,"context_line":"     */"},{"line_number":60,"context_line":"    public function get($uri, array $options \u003d []);"},{"line_number":61,"context_line":""}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_861367b7","line":58,"in_reply_to":"1ae5cdf2_79f9d04c","updated":"2014-06-02 09:49:23.000000000","message":"Done","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":8512,"name":"Matt Farina","email":"matt@mattfarina.com","username":"mattfarina"},"change_message_id":"53b452c4da5af548b41bb6be8040d54c07a809d7","unresolved":false,"context_lines":[{"line_number":72,"context_line":"     * Execute a POST request."},{"line_number":73,"context_line":"     *"},{"line_number":74,"context_line":"     * @param       $uri     URL the request will send to"},{"line_number":75,"context_line":"     * @param       $body    Entity body being sent"},{"line_number":76,"context_line":"     * @param array $options Configuration options, such as headers"},{"line_number":77,"context_line":"     * @return ResponseInterface"},{"line_number":78,"context_line":"     */"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_2c10e84d","line":75,"updated":"2014-05-30 15:40:23.000000000","message":"What types can the body be? They should be listed and documented here with pointers to things that need more detail such as streams. This applies to put() as well.","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"815d0ead77310fa1a8d4e1f105cfd4a2d6564a12","unresolved":false,"context_lines":[{"line_number":72,"context_line":"     * Execute a POST request."},{"line_number":73,"context_line":"     *"},{"line_number":74,"context_line":"     * @param       $uri     URL the request will send to"},{"line_number":75,"context_line":"     * @param       $body    Entity body being sent"},{"line_number":76,"context_line":"     * @param array $options Configuration options, such as headers"},{"line_number":77,"context_line":"     * @return ResponseInterface"},{"line_number":78,"context_line":"     */"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_06275713","line":75,"in_reply_to":"1ae5cdf2_2c10e84d","updated":"2014-06-02 09:49:23.000000000","message":"Same with the URL, I don\u0027t think we should constrain on the input type here. Once we\u0027ve built a stream or entity class, this will also handle the data type logic. All this method is doing is passing the buck on to these dedicated classes.","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":8512,"name":"Matt Farina","email":"matt@mattfarina.com","username":"mattfarina"},"change_message_id":"53b452c4da5af548b41bb6be8040d54c07a809d7","unresolved":false,"context_lines":[{"line_number":114,"context_line":"     * @param string $key The option name"},{"line_number":115,"context_line":"     * @return mixed|null"},{"line_number":116,"context_line":"     */"},{"line_number":117,"context_line":"    public function getOption($key);"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    /**"},{"line_number":120,"context_line":"     * Returns the base URL that the client points towards."}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_6c74b0fd","line":117,"updated":"2014-05-30 15:40:23.000000000","message":"I\u0027m not sure I like this because the response for nothing being set is null and a possible set response is null.\n\nIf you look at the config and hasConfig methods in bootstrap (https://github.com/stackforge/openstack-sdk-php/blob/153e6e8b678c912dacf3ac303fbf32ae4f8fe378/src/OpenStack/Bootstrap.php#L181) you\u0027ll see a slightly different style. config accepts a key and a default to use if an option isn\u0027t found.\n\nWhat do you think about breaking this up into the methods getOption with a default value and a hasOption.","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"815d0ead77310fa1a8d4e1f105cfd4a2d6564a12","unresolved":false,"context_lines":[{"line_number":114,"context_line":"     * @param string $key The option name"},{"line_number":115,"context_line":"     * @return mixed|null"},{"line_number":116,"context_line":"     */"},{"line_number":117,"context_line":"    public function getOption($key);"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    /**"},{"line_number":120,"context_line":"     * Returns the base URL that the client points towards."}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_a6f2236d","line":117,"in_reply_to":"1ae5cdf2_6c74b0fd","updated":"2014-06-02 09:49:23.000000000","message":"Why pass in a default when you can apply that functionality outside of the scope of the method itself?\n\n$option \u003d $client-\u003egetOption(\u0027foo\u0027) ?: \u0027default_value\u0027;\n\nIMO, setting a default isn\u0027t expected functionality for a \"get config\" method. If a value has not been set, it should return NULL - because all you\u0027re doing is returning previously set values.\n\nI like the idea of a `hasOption` method","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":8512,"name":"Matt Farina","email":"matt@mattfarina.com","username":"mattfarina"},"change_message_id":"53b452c4da5af548b41bb6be8040d54c07a809d7","unresolved":false,"context_lines":[{"line_number":121,"context_line":"     *"},{"line_number":122,"context_line":"     * @return string"},{"line_number":123,"context_line":"     */"},{"line_number":124,"context_line":"    public function getBaseUrl();"},{"line_number":125,"context_line":"}"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_6c55901c","line":124,"range":{"start_line":124,"start_character":20,"end_line":124,"end_character":30},"updated":"2014-05-30 15:40:23.000000000","message":"What is the case for this being used? I don\u0027t see the next level up using it. Shouldn\u0027t we only add methods to interfaces when they are needed by a caller?","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"815d0ead77310fa1a8d4e1f105cfd4a2d6564a12","unresolved":false,"context_lines":[{"line_number":121,"context_line":"     *"},{"line_number":122,"context_line":"     * @return string"},{"line_number":123,"context_line":"     */"},{"line_number":124,"context_line":"    public function getBaseUrl();"},{"line_number":125,"context_line":"}"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_c6a00f46","line":124,"in_reply_to":"1ae5cdf2_6c55901c","updated":"2014-06-02 09:49:23.000000000","message":"Services will repeatedly use this method because they need to know what URL they\u0027re hitting for API operations. This is a property of the transport client, which each service has their own version of.\n\nWhen a new service is created, this process happens:\n\n1. Authentication call returns service catalog\n\n2. A base URL is found using service-type, service-name and region\n\n3. That value is used to create a new transport client\n\n4. A service is created, and the transport client is injected into the service\n\nWhen a service makes an API call, it will do something like:\n\n$url \u003d $this-\u003etransport-\u003egetBaseUrl();\n\n$url-\u003eaddPath(\u0027servers\u0027);","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"809c3b64acb8c0d197a9a82286db9dfdd7aded5d","unresolved":false,"context_lines":[{"line_number":121,"context_line":"     *"},{"line_number":122,"context_line":"     * @return string"},{"line_number":123,"context_line":"     */"},{"line_number":124,"context_line":"    public function getBaseUrl();"},{"line_number":125,"context_line":"}"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_a049ab32","line":124,"in_reply_to":"1ae5cdf2_8a8060c1","updated":"2014-06-05 08:04:26.000000000","message":"Because you\u0027re introducing global state. This global transport client would act like a singleton and produce unknown side effects on other services. \n\nIf transport clients are instantiated with fresh data, catered specifically for that service client, we have a tailored/specific instance for use. We\u0027d be absolutely certain that changes made to this transport client would not affect another service using it. \n\nFrom my perspective, services need to be isolated - they shouldn\u0027t be tightly coupled to a dependency that\u0027s easily affected by others. It makes the whole system brittle.\n\nIMO - there are only downsides to using singletons.","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":8512,"name":"Matt Farina","email":"matt@mattfarina.com","username":"mattfarina"},"change_message_id":"c916c6f38fcc88f3944ca78de07b25bd5c90b9b6","unresolved":false,"context_lines":[{"line_number":121,"context_line":"     *"},{"line_number":122,"context_line":"     * @return string"},{"line_number":123,"context_line":"     */"},{"line_number":124,"context_line":"    public function getBaseUrl();"},{"line_number":125,"context_line":"}"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_81e4a7ac","line":124,"in_reply_to":"1ae5cdf2_a049ab32","updated":"2014-06-05 14:58:17.000000000","message":"Two things...\n\n1) This wouldn\u0027t be a singleton because you could it\u0027s injectable. You could use the same one for all of your connections, you could use one for each service, or anything in between. This is about flexibility. There is no global state.\n\n2) The transport client should be about the transport of data. Why should it have any state about a service. The service client contains the data about the service. The transport client knows now to communicate. They have separate concerns. If the transport client knows about the service than state is being leaked from one thing to another.","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":8512,"name":"Matt Farina","email":"matt@mattfarina.com","username":"mattfarina"},"change_message_id":"41fde69aa7a7152075a8f0334006ccf95c08ac16","unresolved":false,"context_lines":[{"line_number":121,"context_line":"     *"},{"line_number":122,"context_line":"     * @return string"},{"line_number":123,"context_line":"     */"},{"line_number":124,"context_line":"    public function getBaseUrl();"},{"line_number":125,"context_line":"}"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_8a8060c1","line":124,"in_reply_to":"1ae5cdf2_c6a00f46","updated":"2014-06-04 17:47:32.000000000","message":"I don\u0027t know what we\u0027re imagining the same thing here. You could have the process be.\n\n1. Authentication call returns service catalog\n2. A base URL is found using service-type, service-name and region.\n3. Use an existing transport client to call to Service A.\n4. Use an existing transport client to call to Service B.\n5. Use an existing transport client to call to Service A.\n\nYou could pass around the same transport client to all calls in a page request using just one object.\n\nWhy would we want to limit that? Why create so many transport clients when we can just pass one around?","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"}],"src/OpenStack/Common/Transport/Exception/AuthorizationException.php":[{"author":{"_account_id":8512,"name":"Matt Farina","email":"matt@mattfarina.com","username":"mattfarina"},"change_message_id":"53b452c4da5af548b41bb6be8040d54c07a809d7","unresolved":false,"context_lines":[{"line_number":20,"context_line":"use OpenStack\\Common\\Exception;"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"/**"},{"line_number":23,"context_line":" * Represents an authorization failure."},{"line_number":24,"context_line":" */"},{"line_number":25,"context_line":"class AuthorizationException extends Exception"},{"line_number":26,"context_line":"{"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_eff8b2e7","line":23,"updated":"2014-05-30 15:40:23.000000000","message":"It might be a good idea to keep the HTTP status codes in here. This could be useful when debugging. If I end up in the docs for an API call it will tell me about HTTP messages with numbers.\n\nWhat do you think of a second sentence like \"For example, an HTTP 403 response.\"\n\nI noticed this because I\u0027ve popped open this exception to read the comment that the HTTP number have been helpful.","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"0833f80683a11e5966c4d34b98248bee30dd5e35","unresolved":false,"context_lines":[{"line_number":20,"context_line":"use OpenStack\\Common\\Exception;"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"/**"},{"line_number":23,"context_line":" * Represents an authorization failure."},{"line_number":24,"context_line":" */"},{"line_number":25,"context_line":"class AuthorizationException extends Exception"},{"line_number":26,"context_line":"{"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_230137af","line":23,"in_reply_to":"1ae5cdf2_6abeb466","updated":"2014-06-04 19:05:12.000000000","message":"I don\u0027t think this class actually serves a purpose - shall I remove it?","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":8512,"name":"Matt Farina","email":"matt@mattfarina.com","username":"mattfarina"},"change_message_id":"41fde69aa7a7152075a8f0334006ccf95c08ac16","unresolved":false,"context_lines":[{"line_number":20,"context_line":"use OpenStack\\Common\\Exception;"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"/**"},{"line_number":23,"context_line":" * Represents an authorization failure."},{"line_number":24,"context_line":" */"},{"line_number":25,"context_line":"class AuthorizationException extends Exception"},{"line_number":26,"context_line":"{"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_6abeb466","line":23,"in_reply_to":"1ae5cdf2_c63c8fe5","updated":"2014-06-04 17:47:32.000000000","message":"The other two exceptions extend this one. Not sure I have an opinion on this way of doing things.","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"815d0ead77310fa1a8d4e1f105cfd4a2d6564a12","unresolved":false,"context_lines":[{"line_number":20,"context_line":"use OpenStack\\Common\\Exception;"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"/**"},{"line_number":23,"context_line":" * Represents an authorization failure."},{"line_number":24,"context_line":" */"},{"line_number":25,"context_line":"class AuthorizationException extends Exception"},{"line_number":26,"context_line":"{"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_c63c8fe5","line":23,"in_reply_to":"1ae5cdf2_eff8b2e7","updated":"2014-06-02 09:49:23.000000000","message":"I\u0027m not really sure why this exception class is needed - it seems to act like a general exception for 401 and 403 errors. But we already have specific exception classes for each - which is why I removed any reference to the status codes in the docblock.\n\nIs there any reason to keep this class?","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"}],"src/OpenStack/Common/Transport/Exception/ConflictException.php":[{"author":{"_account_id":9700,"name":"Sam Choi","email":"sam.choi@hp.com","username":"samuelchoi"},"change_message_id":"edaa0e401ba29dd9be4caacd2eeeb425f6968031","unresolved":false,"context_lines":[{"line_number":18,"context_line":"namespace OpenStack\\Common\\Transport\\Exception;"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"/**"},{"line_number":21,"context_line":" * Exception that represents a 409 Conflict HTTP error."},{"line_number":22,"context_line":" *"},{"line_number":23,"context_line":" * This class is thrown when a request could not be completed due to a conflict"},{"line_number":24,"context_line":" * with the current state of the API resource."}],"source_content_type":"text/x-php","patch_set":1,"id":"7adec928_3c78bfc3","line":21,"updated":"2014-05-14 10:14:52.000000000","message":"I like how you\u0027ve expanded to describe the HTTP error codes, but why would you remove the examples that were previously mentioned? While creating APIs (OpenStack or other), my experience has been that interpreting vague HTTP error codes is always unpleasant so I can certainly appreciate the old and new text.","commit_id":"2fa3167d72667bdc10d7964ce43aa29df058d9e1"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"d0d9f99f5966428f830f03e642a23d905aaf4b39","unresolved":false,"context_lines":[{"line_number":18,"context_line":"namespace OpenStack\\Common\\Transport\\Exception;"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"/**"},{"line_number":21,"context_line":" * Exception that represents a 409 Conflict HTTP error."},{"line_number":22,"context_line":" *"},{"line_number":23,"context_line":" * This class is thrown when a request could not be completed due to a conflict"},{"line_number":24,"context_line":" * with the current state of the API resource."}],"source_content_type":"text/x-php","patch_set":1,"id":"7adec928_179e7c56","line":21,"in_reply_to":"7adec928_3c78bfc3","updated":"2014-05-14 10:51:36.000000000","message":"I see where you\u0027re coming from. I\u0027m not against examples, I just didn\u0027t want to restrict these errors to a particular case. Instead I wanted them to be as applicable as possible.\n\nBut I agree: some of the terminology used is a bit ambiguous - so I\u0027ll add in a concrete example to the docblock description of each exception.","commit_id":"2fa3167d72667bdc10d7964ce43aa29df058d9e1"}],"src/OpenStack/Common/Transport/Exception/RequestException.php":[{"author":{"_account_id":8512,"name":"Matt Farina","email":"matt@mattfarina.com","username":"mattfarina"},"change_message_id":"53b452c4da5af548b41bb6be8040d54c07a809d7","unresolved":false,"context_lines":[{"line_number":42,"context_line":"     */"},{"line_number":43,"context_line":"    public function __construct($errorMessage, RequestInterface $request, ResponseInterface $response)"},{"line_number":44,"context_line":"    {"},{"line_number":45,"context_line":"        parent::__construct($errorMessage);"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"        $this-\u003erequest  \u003d $request;"},{"line_number":48,"context_line":"        $this-\u003eresponse \u003d $response;"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_0a686cb9","line":45,"updated":"2014-05-30 15:40:23.000000000","message":"I just had a thought I wanted to bounce around. The second argument for the parent constructor is the code. Should we pass in the http response code here?\n\nhttp://us2.php.net/manual/en/exception.construct.php","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"815d0ead77310fa1a8d4e1f105cfd4a2d6564a12","unresolved":false,"context_lines":[{"line_number":42,"context_line":"     */"},{"line_number":43,"context_line":"    public function __construct($errorMessage, RequestInterface $request, ResponseInterface $response)"},{"line_number":44,"context_line":"    {"},{"line_number":45,"context_line":"        parent::__construct($errorMessage);"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"        $this-\u003erequest  \u003d $request;"},{"line_number":48,"context_line":"        $this-\u003eresponse \u003d $response;"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_6119e578","line":45,"in_reply_to":"1ae5cdf2_0a686cb9","updated":"2014-06-02 09:49:23.000000000","message":"Yeah I like this idea - I\u0027ll add it in","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":10161,"name":"Shaunak Kashyap","email":"shaunak.kashyap@rackspace.com","username":"ycombinator"},"change_message_id":"5e388eb90c003798b533b480a7aaa7d5f4c1fb6d","unresolved":false,"context_lines":[{"line_number":107,"context_line":"    {"},{"line_number":108,"context_line":"        return $this-\u003erequest;"},{"line_number":109,"context_line":"    }"},{"line_number":110,"context_line":"} "}],"source_content_type":"text/x-php","patch_set":11,"id":"1ae5cdf2_304da4c5","line":110,"updated":"2014-06-07 00:34:45.000000000","message":"Minor issue: remove trailing whitespace.","commit_id":"c91e2932aa6dfd9813a61f70698eb7a438306af8"}],"src/OpenStack/Common/Transport/Guzzle/GuzzleAdapter.php":[{"author":{"_account_id":8512,"name":"Matt Farina","email":"matt@mattfarina.com","username":"mattfarina"},"change_message_id":"cdff1cdd05f524a398e39fcc31aec482c2cd1e4f","unresolved":false,"context_lines":[{"line_number":31,"context_line":" * {@see OpenStack\\Common\\Transport\\ClientInterface}, acting as an intermediary"},{"line_number":32,"context_line":" * between Guzzle and our interface, reconciling their differences."},{"line_number":33,"context_line":" */"},{"line_number":34,"context_line":"class GuzzleAdapter extends AbstractClient"},{"line_number":35,"context_line":"{"},{"line_number":36,"context_line":"    /**"},{"line_number":37,"context_line":"     * @var Client The client being wrapped."}],"source_content_type":"text/x-php","patch_set":1,"id":"7adec928_9d3d63be","line":34,"updated":"2014-05-13 03:42:57.000000000","message":"Why is the naming pattern Adapter used here?\n\nThis is a REST client that implements the client interface. It\u0027s not implementing the adapter software pattern (e.g., http://www.fluffycat.com/PHP-Design-Patterns/Adapter/). Why is the name adapter being used?\n\nIt may be a difference in the way we look at it. Are you seeing this as adapting guzzle to work with the transport client interface? I\u0027m seeing this as being a client that implements the interface that happens to use guzzle (an implementation detail). It\u0027s not about guzzle or trying to be like guzzle. It\u0027s an interface that is the minimum needed for the SDK and a simple implementation of that interface that happens to use guzzle.","commit_id":"2fa3167d72667bdc10d7964ce43aa29df058d9e1"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"866e714f4d874b3acfcfb855343f6d293e62a4a8","unresolved":false,"context_lines":[{"line_number":31,"context_line":" * {@see OpenStack\\Common\\Transport\\ClientInterface}, acting as an intermediary"},{"line_number":32,"context_line":" * between Guzzle and our interface, reconciling their differences."},{"line_number":33,"context_line":" */"},{"line_number":34,"context_line":"class GuzzleAdapter extends AbstractClient"},{"line_number":35,"context_line":"{"},{"line_number":36,"context_line":"    /**"},{"line_number":37,"context_line":"     * @var Client The client being wrapped."}],"source_content_type":"text/x-php","patch_set":1,"id":"7adec928_4e707fa5","line":34,"in_reply_to":"7adec928_9d3d63be","updated":"2014-05-13 11:06:40.000000000","message":"If we look at this class and think about its responsibilities, it\u0027s not actually a client. It doesn\u0027t directly create Request objects, send Response objects, etc. What it\u0027s doing instead is _invoking_ Guzzle\u0027s methods, which then handles all the HTTP logic. \n\nI agree with you, though - the interface does not care about implementation details, and nor should it. We wouldn\u0027t need this adapter if Guzzle\u0027s HTTP client implemented the same interface as our HTTP client. But because they\u0027re incompatible interfaces, we need some way to bridge them.\n\nAll this adapter class does is act as a middleman between OUR interface and Guzzle\u0027s. By connecting the dots and patching up dissimilarities, it makes Guzzle satisfy our interface, and that\u0027s all we care about as consumers. But the fact remains that this class is not actually a HTTP client - it doesn\u0027t handle any HTTP logic internally. It\u0027s just an old-fashioned switchboard operator that connects the calls to the right jack. The operator doesn\u0027t participate with or contribute to the actual conversation. To outsiders, as far as they\u0027re concerned, they\u0027re just interacting with OpenStack. \n\nOne of the most important design principles that we want from this SDK is the ability to inject HTTP clients. What this means is that we cannot be tightly couple to one single implementation - we should not have *one* overall HTTP client that handles different underlying libraries differently. All that we require are classes that implement our interface - and this is what this adapter does: the only responsibility it has is making Guzzle satisfy our interface, nothing more.\n\nIf a user wants to inject their own bespoke HTTP client - that\u0027s fine, they wouldn\u0027t need an adapter. All they\u0027d need to do is make sure their class implements our interface (to satisfy all our typehints). You ONLY need adapters for *existing* classes (in 3rd party codebases) that do not implement our interface - like Symfony, Zend Framework 2, etc.","commit_id":"2fa3167d72667bdc10d7964ce43aa29df058d9e1"},{"author":{"_account_id":9700,"name":"Sam Choi","email":"sam.choi@hp.com","username":"samuelchoi"},"change_message_id":"edaa0e401ba29dd9be4caacd2eeeb425f6968031","unresolved":false,"context_lines":[{"line_number":45,"context_line":"     * such as overriding error handling and the default User-Agent header."},{"line_number":46,"context_line":"     *"},{"line_number":47,"context_line":"     * @param array $options The options passed in to the Guzzle client"},{"line_number":48,"context_line":"     * @return GuzzleAdapter"},{"line_number":49,"context_line":"     */"},{"line_number":50,"context_line":"    public static function create(array $options \u003d [])"},{"line_number":51,"context_line":"    {"}],"source_content_type":"text/x-php","patch_set":1,"id":"7adec928_576e4497","line":48,"updated":"2014-05-14 10:14:52.000000000","message":"Since the current code base is using fully qualified class names in comments, why use the short/local class name GuzzleAdapter? \n\nAlso, the last time I looked into using fqcn vs short names, it appeared that not all IDEs actually supported auto-complete for short names. Since we can\u0027t assume a particular IDE/editor, it seems that using fqcn\u0027s would be the safe bet. \n\nhttp://stackoverflow.com/questions/10710596/right-syntax-to-get-auto-complete-in-ide-thanks-to-phpdocumentor","commit_id":"2fa3167d72667bdc10d7964ce43aa29df058d9e1"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"d0d9f99f5966428f830f03e642a23d905aaf4b39","unresolved":false,"context_lines":[{"line_number":45,"context_line":"     * such as overriding error handling and the default User-Agent header."},{"line_number":46,"context_line":"     *"},{"line_number":47,"context_line":"     * @param array $options The options passed in to the Guzzle client"},{"line_number":48,"context_line":"     * @return GuzzleAdapter"},{"line_number":49,"context_line":"     */"},{"line_number":50,"context_line":"    public static function create(array $options \u003d [])"},{"line_number":51,"context_line":"    {"}],"source_content_type":"text/x-php","patch_set":1,"id":"7adec928_d716d4a6","line":48,"in_reply_to":"7adec928_576e4497","updated":"2014-05-14 10:51:36.000000000","message":"I was really unsure about this - I keep seeing FQCN and short forms being used differently in other projects. So I ended up asking Mike van Riel, one of phpdoc\u0027s lead devs, on Twitter (@mvriel) about this and he said:\n\n\"docblocks support the same things as php. So you can use imports and have to prefix a fqcn with a \\\"\n\nhttps://twitter.com/mvriel/status/463991788156366848\n\nSo either one is supported. If it makes the lives easier of IDE users, I guess that pushes use more towards using FQCN. I don\u0027t mind either way as long as we\u0027re consistent. I like the advantage of shorter forms because it\u0027s less words and doesn\u0027t crowd the docblocks.\n\nDoes anybody have a strong opinion about either one? I use phpstorm which is okay with imported classes","commit_id":"2fa3167d72667bdc10d7964ce43aa29df058d9e1"},{"author":{"_account_id":8512,"name":"Matt Farina","email":"matt@mattfarina.com","username":"mattfarina"},"change_message_id":"886a7f1acea1f4b57f9874394f027f48e479858b","unresolved":false,"context_lines":[{"line_number":45,"context_line":"     * such as overriding error handling and the default User-Agent header."},{"line_number":46,"context_line":"     *"},{"line_number":47,"context_line":"     * @param array $options The options passed in to the Guzzle client"},{"line_number":48,"context_line":"     * @return GuzzleAdapter"},{"line_number":49,"context_line":"     */"},{"line_number":50,"context_line":"    public static function create(array $options \u003d [])"},{"line_number":51,"context_line":"    {"}],"source_content_type":"text/x-php","patch_set":1,"id":"7adec928_731be680","line":48,"in_reply_to":"7adec928_d716d4a6","updated":"2014-05-19 17:28:01.000000000","message":"I think we need to consider the developer experience of someone using the SDK to get some work done. That\u0027s a primary user and IDEs are common. We take on some discomfort to help them be more successful.","commit_id":"2fa3167d72667bdc10d7964ce43aa29df058d9e1"},{"author":{"_account_id":8512,"name":"Matt Farina","email":"matt@mattfarina.com","username":"mattfarina"},"change_message_id":"53b452c4da5af548b41bb6be8040d54c07a809d7","unresolved":false,"context_lines":[{"line_number":44,"context_line":"     * This method also handles the configuration of the client being wrapped,"},{"line_number":45,"context_line":"     * such as overriding error handling and the default User-Agent header."},{"line_number":46,"context_line":"     *"},{"line_number":47,"context_line":"     * @param array $options The options passed in to the Guzzle client"},{"line_number":48,"context_line":"     * @return self"},{"line_number":49,"context_line":"     */"},{"line_number":50,"context_line":"    public static function create(array $options \u003d [])"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_b9d8b854","line":47,"updated":"2014-05-30 15:40:23.000000000","message":"Can you please document the options or point to where they are documented. We should make it easy for someone to figure out how to modify them.","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"815d0ead77310fa1a8d4e1f105cfd4a2d6564a12","unresolved":false,"context_lines":[{"line_number":44,"context_line":"     * This method also handles the configuration of the client being wrapped,"},{"line_number":45,"context_line":"     * such as overriding error handling and the default User-Agent header."},{"line_number":46,"context_line":"     *"},{"line_number":47,"context_line":"     * @param array $options The options passed in to the Guzzle client"},{"line_number":48,"context_line":"     * @return self"},{"line_number":49,"context_line":"     */"},{"line_number":50,"context_line":"    public static function create(array $options \u003d [])"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_21d99d83","line":47,"in_reply_to":"1ae5cdf2_b9d8b854","updated":"2014-06-02 09:49:23.000000000","message":"Done","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":8512,"name":"Matt Farina","email":"matt@mattfarina.com","username":"mattfarina"},"change_message_id":"53b452c4da5af548b41bb6be8040d54c07a809d7","unresolved":false,"context_lines":[{"line_number":74,"context_line":"        $this-\u003eclient \u003d $guzzle;"},{"line_number":75,"context_line":"    }"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"    public function createRequest($method, $uri \u003d null, $body \u003d null, array $options \u003d [])"},{"line_number":78,"context_line":"    {"},{"line_number":79,"context_line":"        $headers \u003d isset($options[\u0027headers\u0027]) ? $options[\u0027headers\u0027] : [];"},{"line_number":80,"context_line":""}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_4506e721","line":77,"updated":"2014-05-30 15:40:23.000000000","message":"Can we get an inheritdoc header comment?","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"815d0ead77310fa1a8d4e1f105cfd4a2d6564a12","unresolved":false,"context_lines":[{"line_number":74,"context_line":"        $this-\u003eclient \u003d $guzzle;"},{"line_number":75,"context_line":"    }"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"    public function createRequest($method, $uri \u003d null, $body \u003d null, array $options \u003d [])"},{"line_number":78,"context_line":"    {"},{"line_number":79,"context_line":"        $headers \u003d isset($options[\u0027headers\u0027]) ? $options[\u0027headers\u0027] : [];"},{"line_number":80,"context_line":""}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_4104e9bf","line":77,"in_reply_to":"1ae5cdf2_4506e721","updated":"2014-06-02 09:49:23.000000000","message":"You don\u0027t actually need {@inheritDoc} here. You only ever use that annotation where you\u0027re inheriting a docblock (from a parent class or interface) and ADD TO or OVERRIDE the parent:\n\nhttps://twitter.com/mvriel/status/381088407579267072\n\nIf you omit it, it\u0027s inherited by default.","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":8512,"name":"Matt Farina","email":"matt@mattfarina.com","username":"mattfarina"},"change_message_id":"53b452c4da5af548b41bb6be8040d54c07a809d7","unresolved":false,"context_lines":[{"line_number":88,"context_line":""},{"line_number":89,"context_line":"    /**"},{"line_number":90,"context_line":"     * @inheritDoc"},{"line_number":91,"context_line":"     * @codeCoverageIgnore"},{"line_number":92,"context_line":"     * @param \\OpenStack\\Common\\Transport\\RequestInterface $adapter"},{"line_number":93,"context_line":"     * @return \\OpenStack\\Common\\Transport\\ResponseInterface"},{"line_number":94,"context_line":"     * @throws \\OpenStack\\Common\\Transport\\Exception\\RequestException"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_eab980ea","line":91,"updated":"2014-05-30 15:40:23.000000000","message":"Why should this be ignored for code coverage?","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"815d0ead77310fa1a8d4e1f105cfd4a2d6564a12","unresolved":false,"context_lines":[{"line_number":88,"context_line":""},{"line_number":89,"context_line":"    /**"},{"line_number":90,"context_line":"     * @inheritDoc"},{"line_number":91,"context_line":"     * @codeCoverageIgnore"},{"line_number":92,"context_line":"     * @param \\OpenStack\\Common\\Transport\\RequestInterface $adapter"},{"line_number":93,"context_line":"     * @return \\OpenStack\\Common\\Transport\\ResponseInterface"},{"line_number":94,"context_line":"     * @throws \\OpenStack\\Common\\Transport\\Exception\\RequestException"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_81d43121","line":91,"in_reply_to":"1ae5cdf2_eab980ea","updated":"2014-06-02 09:49:23.000000000","message":"Good catch - I added this before implementing mocking. This line can be removed now.","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"}],"src/OpenStack/Common/Transport/Guzzle/HttpError.php":[{"author":{"_account_id":9700,"name":"Sam Choi","email":"sam.choi@hp.com","username":"samuelchoi"},"change_message_id":"5445e2c8da70ae2d0e1bddf3f546c62ccc7160e0","unresolved":false,"context_lines":[{"line_number":39,"context_line":"     * checks for HTTP errors and handles them, this method checks the HTTP"},{"line_number":40,"context_line":"     * status code and invokes {@see RequestException} if necessary."},{"line_number":41,"context_line":"     *"},{"line_number":42,"context_line":"     * @param CompleteEvent $event"},{"line_number":43,"context_line":"     * @throws \\OpenStack\\Common\\Transport\\Exception\\RequestException"},{"line_number":44,"context_line":"     */"},{"line_number":45,"context_line":"    public function onComplete(CompleteEvent $event)"}],"source_content_type":"text/x-php","patch_set":1,"id":"7adec928_745df014","line":42,"updated":"2014-05-15 09:33:15.000000000","message":"As of now, the code base uses fqcn\u0027s whenever classes are documented (@param, @return, @throws etc). Out of curiosity, why are there a few local class names when the rest of the new code you added uses fqcn\u0027s?","commit_id":"2fa3167d72667bdc10d7964ce43aa29df058d9e1"},{"author":{"_account_id":8512,"name":"Matt Farina","email":"matt@mattfarina.com","username":"mattfarina"},"change_message_id":"886a7f1acea1f4b57f9874394f027f48e479858b","unresolved":false,"context_lines":[{"line_number":39,"context_line":"     * checks for HTTP errors and handles them, this method checks the HTTP"},{"line_number":40,"context_line":"     * status code and invokes {@see RequestException} if necessary."},{"line_number":41,"context_line":"     *"},{"line_number":42,"context_line":"     * @param CompleteEvent $event"},{"line_number":43,"context_line":"     * @throws \\OpenStack\\Common\\Transport\\Exception\\RequestException"},{"line_number":44,"context_line":"     */"},{"line_number":45,"context_line":"    public function onComplete(CompleteEvent $event)"}],"source_content_type":"text/x-php","patch_set":1,"id":"7adec928_930a3288","line":42,"in_reply_to":"7adec928_43aae385","updated":"2014-05-19 17:28:01.000000000","message":"Let\u0027s come to a decision quickly. I added an agenda item to the next meeting to discuss FQCNs.","commit_id":"2fa3167d72667bdc10d7964ce43aa29df058d9e1"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"1d3d2ebcb39d0efc1ec87c6dc1608cb2b2d0eed1","unresolved":false,"context_lines":[{"line_number":39,"context_line":"     * checks for HTTP errors and handles them, this method checks the HTTP"},{"line_number":40,"context_line":"     * status code and invokes {@see RequestException} if necessary."},{"line_number":41,"context_line":"     *"},{"line_number":42,"context_line":"     * @param CompleteEvent $event"},{"line_number":43,"context_line":"     * @throws \\OpenStack\\Common\\Transport\\Exception\\RequestException"},{"line_number":44,"context_line":"     */"},{"line_number":45,"context_line":"    public function onComplete(CompleteEvent $event)"}],"source_content_type":"text/x-php","patch_set":1,"id":"7adec928_43aae385","line":42,"in_reply_to":"7adec928_745df014","updated":"2014-05-16 12:46:13.000000000","message":"Most of the functionality of this subscriber was taken from Guzzle, which uses imported short forms. When we reach a decision on whether to use FQCNs or imported versions, I can change this.","commit_id":"2fa3167d72667bdc10d7964ce43aa29df058d9e1"},{"author":{"_account_id":9700,"name":"Sam Choi","email":"sam.choi@hp.com","username":"samuelchoi"},"change_message_id":"5445e2c8da70ae2d0e1bddf3f546c62ccc7160e0","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        $status \u003d (int) $event-\u003egetResponse()-\u003egetStatusCode();"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        // Has an error occurred (4xx or 5xx status)?"},{"line_number":50,"context_line":"        if ($status \u003e\u003d 400 \u0026\u0026 $status \u003c\u003d 505) {"},{"line_number":51,"context_line":"            throw RequestException::create($event-\u003egetRequest(), $event-\u003egetResponse());"},{"line_number":52,"context_line":"        }"},{"line_number":53,"context_line":"    }"}],"source_content_type":"text/x-php","patch_set":1,"id":"7adec928_911a8e7b","line":50,"updated":"2014-05-15 09:33:15.000000000","message":"Maybe it\u0027s covered somewhere else, but in the off-chance that the user gets a 2xx or 3xx error that\u0027s not a 200, what would the user encounter?","commit_id":"2fa3167d72667bdc10d7964ce43aa29df058d9e1"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"1d3d2ebcb39d0efc1ec87c6dc1608cb2b2d0eed1","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        $status \u003d (int) $event-\u003egetResponse()-\u003egetStatusCode();"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        // Has an error occurred (4xx or 5xx status)?"},{"line_number":50,"context_line":"        if ($status \u003e\u003d 400 \u0026\u0026 $status \u003c\u003d 505) {"},{"line_number":51,"context_line":"            throw RequestException::create($event-\u003egetRequest(), $event-\u003egetResponse());"},{"line_number":52,"context_line":"        }"},{"line_number":53,"context_line":"    }"}],"source_content_type":"text/x-php","patch_set":1,"id":"7adec928_438103fa","line":50,"in_reply_to":"7adec928_911a8e7b","updated":"2014-05-16 12:46:13.000000000","message":"You\u0027d never get an HTTP error with 2xx or 3xx status codes. 2xx denote success, 3xx denote redirection - so I can\u0027t ever see a conceivable use-case for handling this range as potential errors.","commit_id":"2fa3167d72667bdc10d7964ce43aa29df058d9e1"},{"author":{"_account_id":10161,"name":"Shaunak Kashyap","email":"shaunak.kashyap@rackspace.com","username":"ycombinator"},"change_message_id":"5e388eb90c003798b533b480a7aaa7d5f4c1fb6d","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        $status \u003d (int) $event-\u003egetResponse()-\u003egetStatusCode();"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        // Has an error occurred (4xx or 5xx status)?"},{"line_number":50,"context_line":"        if ($status \u003e\u003d 400 \u0026\u0026 $status \u003c\u003d 505) {"},{"line_number":51,"context_line":"            $request  \u003d new RequestAdapter($event-\u003egetRequest());"},{"line_number":52,"context_line":"            $response \u003d new ResponseAdapter($event-\u003egetResponse());"},{"line_number":53,"context_line":"            throw RequestException::create($request, $response);"}],"source_content_type":"text/x-php","patch_set":11,"id":"1ae5cdf2_b0e7f4ab","line":50,"updated":"2014-06-07 00:34:45.000000000","message":"Any reason we should limit ourselves to HTTP errors up to 505? I know that\u0027s how many there are today per RFC2616 but what if more are introduced in the future via other RFCs?","commit_id":"c91e2932aa6dfd9813a61f70698eb7a438306af8"},{"author":{"_account_id":10161,"name":"Shaunak Kashyap","email":"shaunak.kashyap@rackspace.com","username":"ycombinator"},"change_message_id":"583838aa28b526faf6bebb3147201df406dceaca","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        $status \u003d (int) $event-\u003egetResponse()-\u003egetStatusCode();"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        // Has an error occurred (4xx or 5xx status)?"},{"line_number":50,"context_line":"        if ($status \u003e\u003d 400 \u0026\u0026 $status \u003c\u003d 505) {"},{"line_number":51,"context_line":"            $request  \u003d new RequestAdapter($event-\u003egetRequest());"},{"line_number":52,"context_line":"            $response \u003d new ResponseAdapter($event-\u003egetResponse());"},{"line_number":53,"context_line":"            throw RequestException::create($request, $response);"}],"source_content_type":"text/x-php","patch_set":11,"id":"1ae5cdf2_02cdba5d","line":50,"in_reply_to":"1ae5cdf2_82baaabe","updated":"2014-06-07 17:15:34.000000000","message":"Fair enough. Done.","commit_id":"c91e2932aa6dfd9813a61f70698eb7a438306af8"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"bd913a5dcc12451b0adbf261d7c2b16f7172a4db","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        $status \u003d (int) $event-\u003egetResponse()-\u003egetStatusCode();"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        // Has an error occurred (4xx or 5xx status)?"},{"line_number":50,"context_line":"        if ($status \u003e\u003d 400 \u0026\u0026 $status \u003c\u003d 505) {"},{"line_number":51,"context_line":"            $request  \u003d new RequestAdapter($event-\u003egetRequest());"},{"line_number":52,"context_line":"            $response \u003d new ResponseAdapter($event-\u003egetResponse());"},{"line_number":53,"context_line":"            throw RequestException::create($request, $response);"}],"source_content_type":"text/x-php","patch_set":11,"id":"1ae5cdf2_82baaabe","line":50,"in_reply_to":"1ae5cdf2_b0e7f4ab","updated":"2014-06-07 17:04:03.000000000","message":"If more HTTP status codes are released, we can submit a new patch adding them. We\u0027d have lots of time and be safe under the assumption that no OpenStack service would immediately adopt them","commit_id":"c91e2932aa6dfd9813a61f70698eb7a438306af8"},{"author":{"_account_id":10161,"name":"Shaunak Kashyap","email":"shaunak.kashyap@rackspace.com","username":"ycombinator"},"change_message_id":"5e388eb90c003798b533b480a7aaa7d5f4c1fb6d","unresolved":false,"context_lines":[{"line_number":53,"context_line":"            throw RequestException::create($request, $response);"},{"line_number":54,"context_line":"        }"},{"line_number":55,"context_line":"    }"},{"line_number":56,"context_line":"} "}],"source_content_type":"text/x-php","patch_set":11,"id":"1ae5cdf2_b0d0d4ca","line":56,"updated":"2014-06-07 00:34:45.000000000","message":"Minor issue: trailing whitespace.","commit_id":"c91e2932aa6dfd9813a61f70698eb7a438306af8"}],"src/OpenStack/Common/Transport/Guzzle/RequestAdapter.php":[{"author":{"_account_id":10161,"name":"Shaunak Kashyap","email":"shaunak.kashyap@rackspace.com","username":"ycombinator"},"change_message_id":"5e388eb90c003798b533b480a7aaa7d5f4c1fb6d","unresolved":false,"context_lines":[{"line_number":51,"context_line":"    {"},{"line_number":52,"context_line":"        $this-\u003emessage-\u003esetUrl($url);"},{"line_number":53,"context_line":"    }"},{"line_number":54,"context_line":"} "}],"source_content_type":"text/x-php","patch_set":11,"id":"1ae5cdf2_d0ec8092","line":54,"updated":"2014-06-07 00:34:45.000000000","message":"Minor issue: trailing whitespace.","commit_id":"c91e2932aa6dfd9813a61f70698eb7a438306af8"}],"src/OpenStack/Common/Transport/Guzzle/ResponseAdapter.php":[{"author":{"_account_id":10161,"name":"Shaunak Kashyap","email":"shaunak.kashyap@rackspace.com","username":"ycombinator"},"change_message_id":"5e388eb90c003798b533b480a7aaa7d5f4c1fb6d","unresolved":false,"context_lines":[{"line_number":42,"context_line":"        return $this-\u003emessage-\u003egetReasonPhrase();"},{"line_number":43,"context_line":"    }"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    public function json()"},{"line_number":46,"context_line":"    {"},{"line_number":47,"context_line":"        return $this-\u003emessage-\u003ejson();"},{"line_number":48,"context_line":"    }"}],"source_content_type":"text/x-php","patch_set":11,"id":"1ae5cdf2_f0009c37","line":45,"updated":"2014-06-07 00:34:45.000000000","message":"Should this be getJson() for consistency and clarity of intent?","commit_id":"c91e2932aa6dfd9813a61f70698eb7a438306af8"}],"src/OpenStack/Common/Transport/MessageInterface.php":[{"author":{"_account_id":10161,"name":"Shaunak Kashyap","email":"shaunak.kashyap@rackspace.com","username":"ycombinator"},"change_message_id":"5e388eb90c003798b533b480a7aaa7d5f4c1fb6d","unresolved":false,"context_lines":[{"line_number":51,"context_line":"     *"},{"line_number":52,"context_line":"     * @throws \\InvalidArgumentException When the body is not valid."},{"line_number":53,"context_line":"     */"},{"line_number":54,"context_line":"    public function setBody(/* StreamInterface */ $body \u003d null);"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    /**"},{"line_number":57,"context_line":"     * Gets all message headers."}],"source_content_type":"text/x-php","patch_set":11,"id":"1ae5cdf2_30fbe448","line":54,"updated":"2014-06-07 00:34:45.000000000","message":"Any reason not to make this an actual type hint by removing the comments around the type?","commit_id":"c91e2932aa6dfd9813a61f70698eb7a438306af8"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"bd913a5dcc12451b0adbf261d7c2b16f7172a4db","unresolved":false,"context_lines":[{"line_number":51,"context_line":"     *"},{"line_number":52,"context_line":"     * @throws \\InvalidArgumentException When the body is not valid."},{"line_number":53,"context_line":"     */"},{"line_number":54,"context_line":"    public function setBody(/* StreamInterface */ $body \u003d null);"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    /**"},{"line_number":57,"context_line":"     * Gets all message headers."}],"source_content_type":"text/x-php","patch_set":11,"id":"1ae5cdf2_a2afa67c","line":54,"in_reply_to":"1ae5cdf2_30fbe448","updated":"2014-06-07 17:04:03.000000000","message":"I don\u0027t think there\u0027s need for a StreamInterface type hint right now because that class doesn\u0027t exist yet. Once it\u0027s introduced we can uncomment the type hint.","commit_id":"c91e2932aa6dfd9813a61f70698eb7a438306af8"},{"author":{"_account_id":10161,"name":"Shaunak Kashyap","email":"shaunak.kashyap@rackspace.com","username":"ycombinator"},"change_message_id":"583838aa28b526faf6bebb3147201df406dceaca","unresolved":false,"context_lines":[{"line_number":51,"context_line":"     *"},{"line_number":52,"context_line":"     * @throws \\InvalidArgumentException When the body is not valid."},{"line_number":53,"context_line":"     */"},{"line_number":54,"context_line":"    public function setBody(/* StreamInterface */ $body \u003d null);"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    /**"},{"line_number":57,"context_line":"     * Gets all message headers."}],"source_content_type":"text/x-php","patch_set":11,"id":"1ae5cdf2_8208ea92","line":54,"in_reply_to":"1ae5cdf2_a2afa67c","updated":"2014-06-07 17:15:34.000000000","message":"Sounds good. Done.","commit_id":"c91e2932aa6dfd9813a61f70698eb7a438306af8"}],"src/OpenStack/Identity/v2/IdentityService.php":[{"author":{"_account_id":9700,"name":"Sam Choi","email":"sam.choi@hp.com","username":"samuelchoi"},"change_message_id":"5445e2c8da70ae2d0e1bddf3f546c62ccc7160e0","unresolved":false,"context_lines":[{"line_number":660,"context_line":"        $headers \u003d ["},{"line_number":661,"context_line":"            \u0027Accept\u0027         \u003d\u003e self::ACCEPT_TYPE,"},{"line_number":662,"context_line":"            \u0027Content-Type\u0027   \u003d\u003e \u0027application/json\u0027,"},{"line_number":663,"context_line":"            \u0027Content-Length\u0027 \u003d\u003e strlen($body)"},{"line_number":664,"context_line":"        ];"},{"line_number":665,"context_line":""},{"line_number":666,"context_line":"        $response \u003d $this-\u003eclient-\u003epost($url, $body, [\u0027headers\u0027 \u003d\u003e $headers]);"}],"source_content_type":"text/x-php","patch_set":1,"id":"7adec928_b43858f0","line":663,"updated":"2014-05-15 09:33:15.000000000","message":"A minor thing - what is everyone\u0027s take on trailing commas in arrays? Currently, the code base does make use of them. I know Drupal/Zend encourage use of trailing commas and standards don\u0027t seem to mention it at all IIRC. \n\nAgain, it\u0027s very minor, but I seem to keep catching these things after having done the PSR-2 update :).","commit_id":"2fa3167d72667bdc10d7964ce43aa29df058d9e1"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"1d3d2ebcb39d0efc1ec87c6dc1608cb2b2d0eed1","unresolved":false,"context_lines":[{"line_number":660,"context_line":"        $headers \u003d ["},{"line_number":661,"context_line":"            \u0027Accept\u0027         \u003d\u003e self::ACCEPT_TYPE,"},{"line_number":662,"context_line":"            \u0027Content-Type\u0027   \u003d\u003e \u0027application/json\u0027,"},{"line_number":663,"context_line":"            \u0027Content-Length\u0027 \u003d\u003e strlen($body)"},{"line_number":664,"context_line":"        ];"},{"line_number":665,"context_line":""},{"line_number":666,"context_line":"        $response \u003d $this-\u003eclient-\u003epost($url, $body, [\u0027headers\u0027 \u003d\u003e $headers]);"}],"source_content_type":"text/x-php","patch_set":1,"id":"7adec928_c31ef3ba","line":663,"in_reply_to":"7adec928_b43858f0","updated":"2014-05-16 12:46:13.000000000","message":"I\u0027m not against using trailing commas if it\u0027s recommended on other projects. I guess the main advantages are the ease of adding future elements and it reduces diff clutter. \n\nThere is, however, a really annoying bug in Xdebug with trailing commas in arrays:\n\nhttps://twitter.com/jeremeamia/status/463426601187700736","commit_id":"2fa3167d72667bdc10d7964ce43aa29df058d9e1"},{"author":{"_account_id":8512,"name":"Matt Farina","email":"matt@mattfarina.com","username":"mattfarina"},"change_message_id":"886a7f1acea1f4b57f9874394f027f48e479858b","unresolved":false,"context_lines":[{"line_number":660,"context_line":"        $headers \u003d ["},{"line_number":661,"context_line":"            \u0027Accept\u0027         \u003d\u003e self::ACCEPT_TYPE,"},{"line_number":662,"context_line":"            \u0027Content-Type\u0027   \u003d\u003e \u0027application/json\u0027,"},{"line_number":663,"context_line":"            \u0027Content-Length\u0027 \u003d\u003e strlen($body)"},{"line_number":664,"context_line":"        ];"},{"line_number":665,"context_line":""},{"line_number":666,"context_line":"        $response \u003d $this-\u003eclient-\u003epost($url, $body, [\u0027headers\u0027 \u003d\u003e $headers]);"}],"source_content_type":"text/x-php","patch_set":1,"id":"7adec928_bea45508","line":663,"in_reply_to":"7adec928_c31ef3ba","updated":"2014-05-19 17:28:01.000000000","message":"I just looked at some code coverage reports with trailing commas and didn\u0027t see a problem. Do we know what xdebug versions have this issue? Is it something to do with the short array syntax? I\u0027m curious of the details.\n\nI prefer training commas because you don\u0027t have to remember to add them in when you add new lines later. It is far more convenient in practice and something I have a bit of experience with from work with JavaScript.","commit_id":"2fa3167d72667bdc10d7964ce43aa29df058d9e1"},{"author":{"_account_id":9700,"name":"Sam Choi","email":"sam.choi@hp.com","username":"samuelchoi"},"change_message_id":"5445e2c8da70ae2d0e1bddf3f546c62ccc7160e0","unresolved":false,"context_lines":[{"line_number":714,"context_line":"        $headers \u003d ["},{"line_number":715,"context_line":"            \u0027Accept\u0027         \u003d\u003e self::ACCEPT_TYPE,"},{"line_number":716,"context_line":"            \u0027Content-Type\u0027   \u003d\u003e \u0027application/json\u0027,"},{"line_number":717,"context_line":"            \u0027Content-Length\u0027 \u003d\u003e strlen($body)"},{"line_number":718,"context_line":"        ];"},{"line_number":719,"context_line":""},{"line_number":720,"context_line":"        $response \u003d $this-\u003eclient-\u003epost($url, $body, [\u0027headers\u0027 \u003d\u003e $headers]);"}],"source_content_type":"text/x-php","patch_set":1,"id":"7adec928_d43964f3","line":717,"updated":"2014-05-15 09:33:15.000000000","message":"Same as previous comment about trailing commas.","commit_id":"2fa3167d72667bdc10d7964ce43aa29df058d9e1"}],"src/OpenStack/ObjectStore/v1/Resource/StreamWrapper.php":[{"author":{"_account_id":9700,"name":"Sam Choi","email":"sam.choi@hp.com","username":"samuelchoi"},"change_message_id":"7819e27d80f1189e907988f518ec8854210a48d9","unresolved":false,"context_lines":[{"line_number":1007,"context_line":"    }"},{"line_number":1008,"context_line":""},{"line_number":1009,"context_line":"    /**"},{"line_number":1010,"context_line":"     *"},{"line_number":1011,"context_line":"     */"},{"line_number":1012,"context_line":"    public function stream_truncate()"},{"line_number":1013,"context_line":"    {"}],"source_content_type":"text/x-php","patch_set":2,"id":"3ae8d1ca_32963bf1","line":1010,"updated":"2014-05-29 09:27:40.000000000","message":"Is this being used at all? I didn\u0027t catch the intent here.","commit_id":"c6a59a153dc4420a4d5be8b0e686a8db387b1a37"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"297e83df2354e85ebfa0188513acad9bb1cd7371","unresolved":false,"context_lines":[{"line_number":1007,"context_line":"    }"},{"line_number":1008,"context_line":""},{"line_number":1009,"context_line":"    /**"},{"line_number":1010,"context_line":"     *"},{"line_number":1011,"context_line":"     */"},{"line_number":1012,"context_line":"    public function stream_truncate()"},{"line_number":1013,"context_line":"    {"}],"source_content_type":"text/x-php","patch_set":2,"id":"3ae8d1ca_8c42933c","line":1010,"in_reply_to":"3ae8d1ca_32963bf1","updated":"2014-05-29 11:05:07.000000000","message":"This is an old patch set. This has been removed in the newest set (4).","commit_id":"c6a59a153dc4420a4d5be8b0e686a8db387b1a37"}],"tests/Tests/Identity/v2/IdentityServicesTest.php":[{"author":{"_account_id":9700,"name":"Sam Choi","email":"sam.choi@hp.com","username":"samuelchoi"},"change_message_id":"5445e2c8da70ae2d0e1bddf3f546c62ccc7160e0","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        $this-\u003eassertNotEmpty($tok);"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"        return $service;"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"    }"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    /**"}],"source_content_type":"text/x-php","patch_set":1,"id":"7adec928_a20736c9","line":107,"updated":"2014-05-15 09:33:15.000000000","message":"What was the reason for removing this test? I think we discussed this particular test in a previous review as well.","commit_id":"2fa3167d72667bdc10d7964ce43aa29df058d9e1"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"1d3d2ebcb39d0efc1ec87c6dc1608cb2b2d0eed1","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        $this-\u003eassertNotEmpty($tok);"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"        return $service;"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"    }"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    /**"}],"source_content_type":"text/x-php","patch_set":1,"id":"7adec928_2368771c","line":107,"in_reply_to":"7adec928_a20736c9","updated":"2014-05-16 12:46:13.000000000","message":"I don\u0027t this patch has been fully rebased against master. For the new patchset, I\u0027ll make sure this test is back in","commit_id":"2fa3167d72667bdc10d7964ce43aa29df058d9e1"},{"author":{"_account_id":8512,"name":"Matt Farina","email":"matt@mattfarina.com","username":"mattfarina"},"change_message_id":"53b452c4da5af548b41bb6be8040d54c07a809d7","unresolved":false,"context_lines":[{"line_number":113,"context_line":"        $username \u003d self::conf(\u0027openstack.identity.username\u0027);"},{"line_number":114,"context_line":"        $password \u003d self::conf(\u0027openstack.identity.password\u0027);"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"        $this-\u003eassertNotEmpty($service-\u003eauthenticateAsUser($username, $password));"},{"line_number":117,"context_line":"    }"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    /**"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_409335fa","side":"PARENT","line":116,"updated":"2014-05-30 15:40:23.000000000","message":"Why was this test removed? It should be left it because it\u0027s a common case to authenticate with just a user/pass.\n\nThis particular test keeps being dropped. Is there a reason for that to keep happening?","commit_id":"7035fa4adebae6433c5f863ecd690bfe3c8acaa6"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"815d0ead77310fa1a8d4e1f105cfd4a2d6564a12","unresolved":false,"context_lines":[{"line_number":113,"context_line":"        $username \u003d self::conf(\u0027openstack.identity.username\u0027);"},{"line_number":114,"context_line":"        $password \u003d self::conf(\u0027openstack.identity.password\u0027);"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"        $this-\u003eassertNotEmpty($service-\u003eauthenticateAsUser($username, $password));"},{"line_number":117,"context_line":"    }"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    /**"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_e1b895e7","side":"PARENT","line":116,"in_reply_to":"1ae5cdf2_409335fa","updated":"2014-06-02 09:49:23.000000000","message":"Sorry - git keeps removing it for some reason, grr!","commit_id":"7035fa4adebae6433c5f863ecd690bfe3c8acaa6"},{"author":{"_account_id":9700,"name":"Sam Choi","email":"sam.choi@hp.com","username":"samuelchoi"},"change_message_id":"47ea5861402a3d9d247e282529d67dac8d56e080","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        $this-\u003eassertNotEmpty($tok);"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"        return $service;"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"    }"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    /**"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_24845dd3","line":107,"updated":"2014-06-01 00:29:03.000000000","message":"Based on all the reviews, it looks like you really don\u0027t like this test :). Can you please add this one back? testAuthenticatingAsUserWithoutTenant()","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"815d0ead77310fa1a8d4e1f105cfd4a2d6564a12","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        $this-\u003eassertNotEmpty($tok);"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"        return $service;"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"    }"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    /**"}],"source_content_type":"text/x-php","patch_set":4,"id":"1ae5cdf2_21ca7d6e","line":107,"in_reply_to":"1ae5cdf2_24845dd3","updated":"2014-06-02 09:49:23.000000000","message":"Ha! Should be added back in for new patch","commit_id":"d039119151e9de9dcc7ee30875542b02c90715e0"},{"author":{"_account_id":9700,"name":"Sam Choi","email":"sam.choi@hp.com","username":"samuelchoi"},"change_message_id":"fdf05a8ece5ce1af6d5a6e2f0619984cd2db66c1","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        $this-\u003eassertNotEmpty($tok);"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"        return $service;"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"    }"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    /**"}],"source_content_type":"text/x-php","patch_set":7,"id":"1ae5cdf2_3288e8e6","line":107,"updated":"2014-06-03 06:11:07.000000000","message":"Looks like we\u0027re missing testAuthenticatingAsUserWithoutTenant() from the base repo.","commit_id":"613cb65f2a743424e495b67c221c4e97aa25b326"}],"tests/Tests/ObjectStore/v1/Resource/StreamWrapperTest.php":[{"author":{"_account_id":9700,"name":"Sam Choi","email":"sam.choi@hp.com","username":"samuelchoi"},"change_message_id":"7819e27d80f1189e907988f518ec8854210a48d9","unresolved":false,"context_lines":[{"line_number":24,"context_line":"/**"},{"line_number":25,"context_line":" * @group streamWrapper"},{"line_number":26,"context_line":" */"},{"line_number":27,"context_line":"class StreamWrapperTest extends StreamWrapperTestCase"},{"line_number":28,"context_line":"{"},{"line_number":29,"context_line":"    const SCHEME \u003d StreamWrapper::DEFAULT_SCHEME;"},{"line_number":30,"context_line":""}],"source_content_type":"text/x-php","patch_set":2,"id":"3ae8d1ca_3b28d733","line":27,"updated":"2014-05-29 09:27:40.000000000","message":"I\u0027ve got a number of related comments about the changes made to StreamWrapperTest and StreamWrapperFSTest:\n\n- While I see that changes you\u0027ve made to these tests primarily involves a bit of light refactoring and the addition of StreamWrapperTestCase (great add btw), why is it part of this patch? The scope was already fairly large to begin with, and this introduces more scope creep. These changes would have been better suited for a separate patch, and it would make it easier for the reviewers to get through this current patch in a more timely manner. \n\n- Why delete old files, add new files, when the types of tests and the intent are essentially the same? Again, this makes it more difficult for reviewers to quickly discern what actually changed between patches since we see new/deleted files instead of a renamed file with modifications that can be tracked.","commit_id":"c6a59a153dc4420a4d5be8b0e686a8db387b1a37"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"297e83df2354e85ebfa0188513acad9bb1cd7371","unresolved":false,"context_lines":[{"line_number":24,"context_line":"/**"},{"line_number":25,"context_line":" * @group streamWrapper"},{"line_number":26,"context_line":" */"},{"line_number":27,"context_line":"class StreamWrapperTest extends StreamWrapperTestCase"},{"line_number":28,"context_line":"{"},{"line_number":29,"context_line":"    const SCHEME \u003d StreamWrapper::DEFAULT_SCHEME;"},{"line_number":30,"context_line":""}],"source_content_type":"text/x-php","patch_set":2,"id":"3ae8d1ca_cc91fb9a","line":27,"in_reply_to":"3ae8d1ca_3b28d733","updated":"2014-05-29 11:05:07.000000000","message":"- In answer to your first point: the only reason this was refactored was to fix the \"segfault\" bug that appeared as a consequence of this patch. There was a memory leak in these test cases, arising from streams not being closed properly and being passed around to different unit tests. In order for the memory leak to be fixed, a refactor was needed where the fixture stream was handled as a class property. I tried to minimize this change as much as possible. I don\u0027t think we can extract this stuff because otherwise this patch would be introducing a bug into the codebase.\n\n- This is an unfortunate consequence of moving files around in an IDE. Sometimes they don\u0027t `git mv`. I\u0027ll make more of an effort in the future to preserve this convention.","commit_id":"c6a59a153dc4420a4d5be8b0e686a8db387b1a37"},{"author":{"_account_id":9700,"name":"Sam Choi","email":"sam.choi@hp.com","username":"samuelchoi"},"change_message_id":"f989791f21d26e13003faa20e4419f743063abda","unresolved":false,"context_lines":[{"line_number":24,"context_line":"/**"},{"line_number":25,"context_line":" * @group streamWrapper"},{"line_number":26,"context_line":" */"},{"line_number":27,"context_line":"class StreamWrapperTest extends StreamWrapperTestCase"},{"line_number":28,"context_line":"{"},{"line_number":29,"context_line":"    const SCHEME \u003d StreamWrapper::DEFAULT_SCHEME;"},{"line_number":30,"context_line":""}],"source_content_type":"text/x-php","patch_set":2,"id":"1ae5cdf2_c44bb9ae","line":27,"in_reply_to":"3ae8d1ca_cc91fb9a","updated":"2014-06-01 00:28:32.000000000","message":"Ok, I\u0027m fine with the changes then since they\u0027re required for the bug fix.\n\nAnd yeah, it\u0027s unfortunate that \u0027git mv\u0027 wasn\u0027t used since we\u0027d be losing the file history and make it more difficult for reviewers to spot the deltas between patches. I\u0027ll do a manual compare this time.","commit_id":"c6a59a153dc4420a4d5be8b0e686a8db387b1a37"},{"author":{"_account_id":9700,"name":"Sam Choi","email":"sam.choi@hp.com","username":"samuelchoi"},"change_message_id":"f989791f21d26e13003faa20e4419f743063abda","unresolved":false,"context_lines":[{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    public function testRegister()"},{"line_number":40,"context_line":"    {"},{"line_number":41,"context_line":"        $this-\u003eassertNotEmpty(StreamWrapper::DEFAULT_SCHEME);"},{"line_number":42,"context_line":"        $this-\u003eassertContains(StreamWrapper::DEFAULT_SCHEME, stream_get_wrappers());"},{"line_number":43,"context_line":"    }"},{"line_number":44,"context_line":""}],"source_content_type":"text/x-php","patch_set":2,"id":"1ae5cdf2_c49019ef","line":41,"updated":"2014-06-01 00:28:32.000000000","message":"I might\u0027ve missed something after the refactoring of the StreamWrapper related tests, but is the call to stream_wrapper_register no longer necessary here?","commit_id":"c6a59a153dc4420a4d5be8b0e686a8db387b1a37"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"58a640ffb123f0fb6f865ed1daa2bcbc4582c72a","unresolved":false,"context_lines":[{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    public function testRegister()"},{"line_number":40,"context_line":"    {"},{"line_number":41,"context_line":"        $this-\u003eassertNotEmpty(StreamWrapper::DEFAULT_SCHEME);"},{"line_number":42,"context_line":"        $this-\u003eassertContains(StreamWrapper::DEFAULT_SCHEME, stream_get_wrappers());"},{"line_number":43,"context_line":"    }"},{"line_number":44,"context_line":""}],"source_content_type":"text/x-php","patch_set":2,"id":"1ae5cdf2_28efbd88","line":41,"in_reply_to":"1ae5cdf2_c49019ef","updated":"2014-06-02 09:41:39.000000000","message":"I agree - it\u0027s not really within the scope of this test case","commit_id":"c6a59a153dc4420a4d5be8b0e686a8db387b1a37"}],"tests/Tests/ObjectStore/v1/StreamWrapperFSTest.php":[{"author":{"_account_id":9700,"name":"Sam Choi","email":"sam.choi@hp.com","username":"samuelchoi"},"change_message_id":"ade5726894242bcbaaa442792047f93d2195e016","unresolved":false,"context_lines":[{"line_number":1,"context_line":"\u003c?php"},{"line_number":2,"context_line":"/* \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":3,"context_line":"(c) Copyright 2012-2014 Hewlett-Packard Development Company, L.P."},{"line_number":4,"context_line":""}],"source_content_type":"text/x-php","patch_set":9,"id":"1ae5cdf2_56eb4d77","line":1,"updated":"2014-06-05 09:11:55.000000000","message":"Seems like something weird happened with this file/patch. Since I\u0027ve already looked at all the previous patches, I\u0027m trying to go through and use patch 8 as the reference version when I look at patch 9 etc so I don\u0027t have to spend time manually finding the deltas. \n\nWhen using base as the reference version, I see the correct StreamWrapperFSTest file. When using patch 8 as the reference version, I see a StreamWrapperFSTest file that\u0027s been added and has 600+ lines (the old one before any of your changes I believe).","commit_id":"2cb9b8ed1ed12d64e9a3eba974dfe87912ca5003"}],"tests/Tests/TestCase.php":[{"author":{"_account_id":9700,"name":"Sam Choi","email":"sam.choi@hp.com","username":"samuelchoi"},"change_message_id":"7819e27d80f1189e907988f518ec8854210a48d9","unresolved":false,"context_lines":[{"line_number":148,"context_line":"                $this-\u003econtainerFixture \u003d $store-\u003econtainer($cname);"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"            }"},{"line_number":151,"context_line":"                // This is why PHP needs \u0027finally\u0027."},{"line_number":152,"context_line":"            catch (\\Exception $e) {"},{"line_number":153,"context_line":"                // Delete the container."},{"line_number":154,"context_line":"                $store-\u003edeleteContainer($cname);"}],"source_content_type":"text/x-php","patch_set":2,"id":"3ae8d1ca_b81ead61","line":151,"updated":"2014-05-29 09:27:40.000000000","message":"Was this an auto-indent?","commit_id":"c6a59a153dc4420a4d5be8b0e686a8db387b1a37"},{"author":{"_account_id":10517,"name":"Jamie Hannaford","email":"jamie.hannaford@rackspace.com","username":"jamiehannaford"},"change_message_id":"297e83df2354e85ebfa0188513acad9bb1cd7371","unresolved":false,"context_lines":[{"line_number":148,"context_line":"                $this-\u003econtainerFixture \u003d $store-\u003econtainer($cname);"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"            }"},{"line_number":151,"context_line":"                // This is why PHP needs \u0027finally\u0027."},{"line_number":152,"context_line":"            catch (\\Exception $e) {"},{"line_number":153,"context_line":"                // Delete the container."},{"line_number":154,"context_line":"                $store-\u003edeleteContainer($cname);"}],"source_content_type":"text/x-php","patch_set":2,"id":"3ae8d1ca_cce81b13","line":151,"in_reply_to":"3ae8d1ca_b81ead61","updated":"2014-05-29 11:05:07.000000000","message":"Good catch. Not sure why this is here - I\u0027ll change","commit_id":"c6a59a153dc4420a4d5be8b0e686a8db387b1a37"}]}