-
Notifications
You must be signed in to change notification settings - Fork 741
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[#714]Fix query parameter construction of org member filter #715
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Sage-Pierce
This looks really good. Minor additions needed.
public void testListMembersWithRole() throws IOException { | ||
GHOrganization org = gitHub.getOrganization(GITHUB_API_TEST_ORG); | ||
|
||
List<GHUser> admins = org.listMembersWithRole("admin").asList(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bug was for listMembersWithFilter
. Can you add a test for that as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
|
||
List<GHUser> admins = org.listMembersWithRole("admin").asList(); | ||
|
||
assertFalse(admins.isEmpty()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Go ahead and be specific about size and who is in the list. A check for non-empty list is not going to be enough to ensure this doesn't break in the future.
Description
Taking a stab at what's mentioned in #714
master
. Create your PR from that branch.mvn -D enable-ci clean install site
locally. This may reformat your code, commit those changes. If this command doesn't succeed, your change will not pass CI.