Skip to content

Removed permission field in createTeam. It is deprecated in the API#619

Merged
bitwiseman merged 1 commit into
hub4j:masterfrom
asthinasthi:deprecate-permission
Nov 20, 2019
Merged

Removed permission field in createTeam. It is deprecated in the API#619
bitwiseman merged 1 commit into
hub4j:masterfrom
asthinasthi:deprecate-permission

Conversation

@asthinasthi

Copy link
Copy Markdown

Description

https://developer.github.com/v3/teams/#create-team had deprecated the permission field but the library still expects this field. https://github.com/github-api/github-api/blob/da11702f6815f658014f33ce724f0cd2e4501081/src/main/java/org/kohsuke/github/GHOrganization.java#L387

This PR refactors the code to remove the permission field.

Before submitting a PR:

We love getting PRs, but we hate asking people for the same basic changes every time.

  • Push your changes to a branch other than master. Create your PR from that branch.
  • Add JavaDocs and other comments
  • Write tests that run and pass in CI. See CONTRIBUTING.md for details on how to capture snapshot data.
  • Run mvn install site locally. This may reformat your code, commit those changes. If this command doesn't succeed, your change will not pass CI.

@bitwiseman

bitwiseman commented Nov 19, 2019

Copy link
Copy Markdown
Member

I rebased your changes on master to make the PR cleaner.

Thanks for this PR it is a good catch. However, removing permissions would be a breaking change, which we will be avoiding until we are ready to create v2.x of this library. So, we can't just remove this field.

Instead you can mark the existing methods that have permissions as @Deprecated (with proper JavaDoc @deprecated note and so on), and create a method that does not use permissions. Also, we'll need tests for both with and without permissions. I've invited you to the github-api-test-org so you can record tests that perform the appropriate actions.

public GHTeam createTeam(String name, Collection<GHRepository> repositories) throws IOException {
     // ...
}
/**
 * ... 
 * @deprecated use {@link #createTeam(String, Collection) instead.
 */
@Deprecated   
public GHTeam createTeam(String name, Permission p, Collection<GHRepository> repositories) throws IOException {
    // ...
}

@bitwiseman bitwiseman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Use @deprecated instead of introducing API changes.

@bitwiseman bitwiseman force-pushed the deprecate-permission branch 2 times, most recently from a81407b to 30662a0 Compare November 19, 2019 04:03
@asthinasthi

asthinasthi commented Nov 19, 2019

Copy link
Copy Markdown
Author

Agreed! It was a long shot to remove those fields.

I just did a hard push on the remote branch & the CI Build job failed. How do I re-trigger it?

@bitwiseman bitwiseman merged commit 7a4870c into hub4j:master Nov 20, 2019
@asthinasthi

Copy link
Copy Markdown
Author

@bitwiseman When is the next release planned?

@bitwiseman

Copy link
Copy Markdown
Member

Done v1.101 is out.

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