Skip to content

Conversation

@jglick
Copy link
Contributor

@jglick jglick commented Dec 16, 2016

JENKINS-36240

Using a new API (currently still in preview) that fixes a longstanding issue.

@reviewbybees

@ghost
Copy link

ghost commented Dec 16, 2016

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

* @param user a {@link GHUser#getLogin}
* @throws FileNotFoundException under some conditions (e.g., private repo you can see but are not an admin of); treat as unknown
* @throws HttpException with a 403 under other conditions (e.g., public repo you have no special rights to); treat as unknown
*/
Copy link
Collaborator

Choose a reason for hiding this comment

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

What if we just have this method return an enum of admin/read/write/none?

Copy link
Collaborator

Choose a reason for hiding this comment

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

And what if the user parameter is GHUser and not `String'?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

have this method return an enum

No problem for me, though we would still need the GHPermission class internally. The current return value seemed more consistent with the overall API design of this component.

what if the user parameter is GHUser

Does not work for the intended use case. We do not have a GHUser object at that point—just the name. To get a GHUser would require another network call.

@kohsuke kohsuke merged commit ccb42d3 into hub4j:master Jan 10, 2017
kohsuke added a commit that referenced this pull request Jan 10, 2017
@jglick jglick deleted the repo-user-permission-JENKINS-36240 branch January 19, 2017 18:35
@jglick
Copy link
Contributor Author

jglick commented Jan 19, 2017

Apparently amended in 6bfeb54, d1c900a, and 911e8d2, and then released in 1.83.

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.

2 participants