Skip to content
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

[#690]Implement the ability to read total_private_repos for GHPerson #691

Merged
merged 1 commit into from
Jan 31, 2020

Conversation

Sage-Pierce
Copy link
Contributor

@Sage-Pierce Sage-Pierce commented Jan 31, 2020

Description

I took a crack at implementing what is mentioned in #690

For test writing, I wasn't sure whether to add to GHPersonTest or GHUserTest. Went with GHUserTest since that's a resource under which you can create a private repo

Prerequisites:

  • 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 formatter:format. This may reformat your code, commit those changes. If the command mvn -D enable-ci install site locally command doesn't succeed, your change will not pass CI.

@Sage-Pierce
Copy link
Contributor Author

Some of the CI steps keep failing on:

File '/home/runner/work/github-api/github-api/src/test/java/org/kohsuke/github/GHUserTest.java' has not been previously formatted. Please format file and commit before running validation!

However, I've run mvn formatter:format locally and committed the result. The validate goal passes locally, but apparently not on CI. Any ideas on why that's the case?

@bitwiseman
Copy link
Member

@Sage-Pierce Are you running on windows?

@Sage-Pierce
Copy link
Contributor Author

Sage-Pierce commented Jan 31, 2020

I'm running on a MacBook (High Sierra)

@bitwiseman bitwiseman merged commit 84dd06d into hub4j:master Jan 31, 2020
@bitwiseman
Copy link
Member

Ugh. That was not what I wanted to do. 😞

@Sage-Pierce Sage-Pierce deleted the #690 branch January 31, 2020 21:59
@Sage-Pierce
Copy link
Contributor Author

Thanks for the (inadvertent) merge 😄

...And subsequent fix for the formatting. Should the formatting be run differently on a MacBook? Just curious what I needed to do to get the correct formatting

@bitwiseman
Copy link
Member

I fixed the instructions: you just need to do mvn clean to make sure the formatter does all the needed formatting.

@Sage-Pierce
Copy link
Contributor Author

@bitwiseman Gotcha

Could you provide an estimate on when there will be a new release containing this change?

Thanks!

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