Implement Rockets endpoint #35
Conversation
***Why?*** - To retrieve rocket's info using the rockets API endpoint provided by SpaceX ***How?*** - Created a Rockets model that exposes an `info` method - This method accepts an optional `rocket_id` - When none passed, it returns the information of all the rockets - Otherwise returns info of the specified rocket id ***Side effects*** - This implementation only makes the info method public whereas other classes in the module are exposing all the methods. This is something that can be fixed in a separate PR
Pull Request Test Coverage Report for Build 60
💛 - Coveralls |
| end | ||
|
|
||
| it "returns all Rockets' when no id is passed info" do | ||
| first_subject = subject.first |
| get(rocket_id) | ||
| end | ||
|
|
||
| private |
There was a problem hiding this comment.
The reason some endpoints have this get or retrieve_all public is that at some point in the past these were exposed as the default methods. So in order to make them backwards compatible, even after we adopted the info and info(foo) as default, I decided to keep carrying them over. You'll probably see people opening PRs using get, retrieve_all instead of info (which is understandable).
I saw your other comments regarding this and I agree with you specially because our release is still at minor versions 0.0.8. Thats the reason I'm not documenting these method on README anymore. I'm planning to release a major version 1.0.0 after closing this list of PRs.
By the way, I got your twitter message. You're more than welcome to help out!
| - `SPACEX::Rockets.info` Retrieve all Rockets; | ||
| - `SPACEX::Rockets.info('falcon1')` Retrieve a specific rocket. Ex: `falcon1` | ||
|
|
||
|
|
There was a problem hiding this comment.
Any reason for skipping the example like we did with all methods? Do you feel that it is an overhead adding examples for each method?
Why?
How?
infomethodrocket_idSide effects/ notes
infomethod public whereas other classes in the gem are exposing all the methods. This is something that I will fix in a separate PR