Skip to content
This repository has been archived by the owner on Dec 18, 2018. It is now read-only.

Provided Organizations API. #34

Merged
merged 3 commits into from
Apr 23, 2017
Merged

Provided Organizations API. #34

merged 3 commits into from
Apr 23, 2017

Conversation

SherifWaly
Copy link
Contributor

PR for #27

@coveralls
Copy link

Coverage Status

Coverage remained the same at 77.273% when pulling f3329a3 on SherifWaly:#27 into 4ef27f2 on decorators-squad:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 77.273% when pulling f3329a3 on SherifWaly:#27 into 4ef27f2 on decorators-squad:master.

@SherifWaly
Copy link
Contributor Author

@amihaiemil Please check this commit :)

@amihaiemil
Copy link
Member

amihaiemil commented Apr 21, 2017

@SherifWaly I'll take a more detailed look tomorrow. Doesn't look bad, but the word is "organization", not "organisation" :D

@SherifWaly
Copy link
Contributor Author

@amihaiemil I found the VersionEye API call it "organisation" but no problem I will change the name :D

@SherifWaly SherifWaly changed the title Provided Organisations API. Provided Organizations API. Apr 21, 2017
@coveralls
Copy link

Coverage Status

Coverage remained the same at 77.273% when pulling 8641d00 on SherifWaly:#27 into 4ef27f2 on decorators-squad:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 77.273% when pulling 8641d00 on SherifWaly:#27 into 4ef27f2 on decorators-squad:master.

Copy link
Member

@amihaiemil amihaiemil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SherifWaly Looks good! Just a few comments, and we don't need OranizationData :D

* @todo #27:30min/DEV Provide RtOrganization as implementation and unit tests.
* The class should work with a given request and organization name.
*/
public interface Organization {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SherifWaly As I see, we won't need OrangizationData, because an Oranziation will have its json behind, from which it will take the api_key, to be able to fetch its teams and projects. Since it will have its json, it will also have accessor methods for the json's attributes.

* @throws IOException If something goes wrong when
* making the HTTP call.
*/
List<OrganizationData> about() throws IOException;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SherifWaly Can you rename this method to fetch()? It's more suitable, I think :D

public interface Organizations {

/**
* Info about Organizations (list of organizations you have access to).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SherifWaly "Fetch the list of oranizations that the authenticated user has access to"

List<OrganizationData> about() throws IOException;

/**
* Fetch a organization.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SherifWaly "Fetch an organization"

@coveralls
Copy link

Coverage Status

Coverage remained the same at 77.273% when pulling 1500253 on SherifWaly:#27 into 4ef27f2 on decorators-squad:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 77.273% when pulling 1500253 on SherifWaly:#27 into 4ef27f2 on decorators-squad:master.

@amihaiemil
Copy link
Member

@SherifWaly nice :D

@amihaiemil
Copy link
Member

@rultor merge

@rultor
Copy link
Collaborator

rultor commented Apr 23, 2017

@rultor merge

@amihaiemil OK, I'll try to merge now. You can check the progress of the merge here

@rultor rultor merged commit 1500253 into decorators-squad:master Apr 23, 2017
@rultor
Copy link
Collaborator

rultor commented Apr 23, 2017

@rultor merge

@amihaiemil Done! FYI, the full log is here (took me 1min)

@SherifWaly SherifWaly deleted the #27 branch April 23, 2017 16:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants