Skip to content

Conversation

@sohtsuka
Copy link
Contributor

Any comments would be highly appreciated (especially on wording).

@boxcla
Copy link

boxcla commented Mar 31, 2015

Verified that @sohtsuka has signed the CLA. Thanks for the pull request!

Copy link
Contributor

Choose a reason for hiding this comment

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

Minor wording suggestion - I think the second sentence should be something like "Can be null to not filter the results."

@gcurtis
Copy link
Contributor

gcurtis commented Mar 31, 2015

Thanks! The code looks really good. I just had a minor suggestion with wording in the docs.

There are also some checkstyle violations that are preventing this from building. If you run gradle build you should be able to reproduce them locally.

:checkstyleTest[ant:checkstyle] /home/travis/build/box/box-java-sdk/src/test/java/com/box/sdk/BoxUserTest.java:13: Wrong order for 'org.hamcrest.Matchers.equalTo' import.
[ant:checkstyle] /home/travis/build/box/box-java-sdk/src/test/java/com/box/sdk/BoxUserTest.java:38:5: File contains tab characters (this is the first instance).
[ant:checkstyle] /home/travis/build/box/box-java-sdk/src/test/java/com/box/sdk/BoxUserTest.java:42: Line has trailing spaces.
[ant:checkstyle] /home/travis/build/box/box-java-sdk/src/test/java/com/box/sdk/BoxUserTest.java:51:78: '+' should be on a new line.
[ant:checkstyle] /home/travis/build/box/box-java-sdk/src/test/java/com/box/sdk/BoxUserTest.java:52: Line is longer than 120 characters (found 125).

@sohtsuka sohtsuka force-pushed the feature/get-all-enterprise-users branch from 5ba5050 to 1eebe20 Compare March 31, 2015 23:03
@sohtsuka
Copy link
Contributor Author

Thanks for reviewing. I have fixed the wording and the checkstyle violations, but when I run gradle build I get following errors:

com.box.sdk.BoxFolderTest > createFolderSendsRequestWithRequiredFields FAILED
    com.box.sdk.BoxAPIException: Couldn't connect to the Box API due to a network error.

com.box.sdk.BoxGroupTest > createGroupSendsCorrectRequestAndParsesResponseCorrectly FAILED
    com.box.sdk.BoxAPIException: Couldn't connect to the Box API due to a network error.

I will investigate later.

@gcurtis
Copy link
Contributor

gcurtis commented Apr 1, 2015

Interesting. I wonder if it's a problem with WireMock. I checked out your branch and built it locally and everything seemed to work. Let me know if you figure out what's causing your build error.

gcurtis added a commit that referenced this pull request Apr 1, 2015
@gcurtis gcurtis merged commit b5d2ac7 into box:master Apr 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants