Add fork branch sync #1898

jonesbusy · 2024-07-17T15:36:49Z

Description

Add support for fork sync branch

Added two wiremock test and tested a SNAPSHOT version in a WIP PR jenkins-infra/plugin-modernizer-tool#141

I confirm this work by enabling logs and ensuring my fork is sync directly on GitHub

(e24ddd0) GitHub API request: POST https://api.github.com/repos/jonesbusy/jobcacher-plugin/merge-upstream

Before submitting a PR:

Changes must not break binary backwards compatibility. If you are unclear on how to make the change you think is needed while maintaining backward compatibility, CONTRIBUTING.md for details.
Add JavaDocs and other comments explaining the behavior.
When adding or updating methods that fetch entities, add @link JavaDoc entries to the relevant documentation on https://docs.github.com/en/rest .
Add tests that cover any added or changed code. This generally requires capturing snapshot test data. See CONTRIBUTING.md for details.
Run mvn -D enable-ci clean install site locally. If this command doesn't succeed, your change will not pass CI.
Push your changes to a branch other than main. You will create your PR from that branch.

When creating a PR:

Fill in the "Description" above with clear summary of the changes. This includes:
If this PR fixes one or more issues, include "Fixes #" lines for each issue.
Provide links to relevant documentation on https://docs.github.com/en/rest where possible. If not including links, explain why not.
All lines of new code should be covered by tests as reported by code coverage. Any lines that are not covered must have PR comments explaining why they cannot be covered. For example, "Reaching this particular exception is hard and is not a particular common scenario."
Enable "Allow edits from maintainers".

API is https://docs.github.com/en/rest/branches/branches?apiVersion=2022-11-28#sync-a-fork-branch-with-the-upstream-repository

Code is covered 100%

codecov · 2024-07-17T17:28:50Z

Codecov Report

Attention: Patch coverage is 92.85714% with 1 line in your changes missing coverage. Please review.

Project coverage is 81.32%. Comparing base (3c4d80c) to head (6493b2c).

Files	Patch %	Lines
src/main/java/org/kohsuke/github/GHBranchSync.java	87.50%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1898      +/-   ##
============================================
+ Coverage     81.29%   81.32%   +0.02%     
- Complexity     2450     2457       +7     
============================================
  Files           238      239       +1     
  Lines          7449     7463      +14     
  Branches        400      400              
============================================
+ Hits           6056     6069      +13     
- Misses         1146     1147       +1     
  Partials        247      247

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/main/java/org/kohsuke/github/GHRepository.java

bitwiseman

Second rounds of feedback, sorry. This is looking good, just a little more to be done.
Thanks!

bitwiseman · 2024-07-24T02:57:23Z

src/main/java/org/kohsuke/github/GHRepository.java

+     * @throws IOException
+     *             the io exception
+     */
+    public GHRepository sync(String branch) throws IOException {


The call actually has a return record that could be useful. Example from the docs:

{ "message": "Successfully fetched and fast-forwarded from upstream defunkt:main", "merge_type": "fast-forward", "base_branch": "defunkt:main" }

Check the schema and return a matching record.

bitwiseman · 2024-07-24T03:04:01Z

src/test/java/org/kohsuke/github/GHRepositoryTest.java

+    @Test
+    public void sync() throws IOException {
+        GHRepository r = getRepository();
+        r.sync("main");


Add slightly more complex tests to cover the returned record and also the error/exception case. (You can do all in one method if you want or separately.)

jonesbusy · 2024-07-24T09:50:08Z

Sure thanks, I will try to add a new model for the sync response and perform some assertion on it

jonesbusy · 2024-07-24T10:40:34Z

I've added the GHBranchSync model and return it

I've also added a negative test when the API return a non-200 status code (for example 422 when the repo is not a fork)

gounthar

Nice, thanks! 👍

NotMyFault

Awesome work!

bitwiseman

Great work on this. Thanks!

bitwiseman · 2024-07-28T05:12:55Z

src/test/java/org/kohsuke/github/GHRepositoryTest.java

+        GHBranchSync sync = r.sync("main");
+        fail("Should have thrown an exception");


There's an assertThrows() method that I prefer for this, but I'm going to block merge for it.

src/main/java/org/kohsuke/github/GHBranchSync.java

jonesbusy · 2024-07-28T09:13:01Z

Apparently something wrong with javadoc. I will take a look

jonesbusy · 2024-07-28T12:51:59Z

Should be fixed, can you restart the workflows please ?

jonesbusy · 2024-08-02T09:10:31Z

Hi @bitwiseman Sorry for the ping, but where you able to review again this PR ?

It's also update with the default branch

Thanks!

jonesbusy · 2024-08-18T05:45:57Z

Thanks for the merge @bitwiseman. Sorry to ping you directly (again) but is there a release soon including this feature ?

I'm asking because we are approaching the end of the Google Summer of Code on our project https://github.com/jenkinsci/plugin-modernizer-tool and would be great that we can include the new release on our delivery

The fork sync is quiet critical on our project to avoid opening PRs from outdated forks.

Thanks for your consideration 😃

bitwiseman · 2024-08-18T20:55:08Z

I'll do a release today.

Add fork branch sync

c40f6b9

gounthar approved these changes Jul 19, 2024

View reviewed changes

bitwiseman reviewed Jul 21, 2024

View reviewed changes

src/main/java/org/kohsuke/github/GHRepository.java Outdated Show resolved Hide resolved

Remove check for fork sync and let the API rethrow exception

5083df7

bitwiseman requested changes Jul 24, 2024

View reviewed changes

Add GHBranchSync model

189125c

jonesbusy requested review from bitwiseman and gounthar July 24, 2024 10:40

gounthar approved these changes Jul 24, 2024

View reviewed changes

NotMyFault approved these changes Jul 27, 2024

View reviewed changes

bitwiseman approved these changes Jul 28, 2024

View reviewed changes

Update src/main/java/org/kohsuke/github/GHBranchSync.java

72f4b76

Add missing javadoc

f98355f

jonesbusy requested a review from bitwiseman July 28, 2024 09:25

Merge branch 'main' into feature/fork-sync

6493b2c

jonesbusy mentioned this pull request Aug 2, 2024

Add fork sync jenkins-infra/plugin-modernizer-tool#141

Merged

6 tasks

bitwiseman merged commit af5bde7 into hub4j:main Aug 5, 2024
12 checks passed

jonesbusy deleted the feature/fork-sync branch August 5, 2024 16:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add fork branch sync #1898

Add fork branch sync #1898

jonesbusy commented Jul 17, 2024 •

edited

Loading

codecov bot commented Jul 17, 2024 •

edited

Loading

bitwiseman left a comment

bitwiseman Jul 24, 2024

bitwiseman Jul 24, 2024

jonesbusy commented Jul 24, 2024

jonesbusy commented Jul 24, 2024

gounthar left a comment

NotMyFault left a comment

bitwiseman left a comment

bitwiseman Jul 28, 2024

jonesbusy commented Jul 28, 2024

jonesbusy commented Jul 28, 2024

jonesbusy commented Aug 2, 2024

jonesbusy commented Aug 18, 2024

bitwiseman commented Aug 18, 2024

		GHBranchSync sync = r.sync("main");
		fail("Should have thrown an exception");

Add fork branch sync #1898

Add fork branch sync #1898

Conversation

jonesbusy commented Jul 17, 2024 • edited Loading

Description

Before submitting a PR:

When creating a PR:

codecov bot commented Jul 17, 2024 • edited Loading

Codecov Report

bitwiseman left a comment

Choose a reason for hiding this comment

bitwiseman Jul 24, 2024

Choose a reason for hiding this comment

bitwiseman Jul 24, 2024

Choose a reason for hiding this comment

jonesbusy commented Jul 24, 2024

jonesbusy commented Jul 24, 2024

gounthar left a comment

Choose a reason for hiding this comment

NotMyFault left a comment

Choose a reason for hiding this comment

bitwiseman left a comment

Choose a reason for hiding this comment

bitwiseman Jul 28, 2024

Choose a reason for hiding this comment

jonesbusy commented Jul 28, 2024

jonesbusy commented Jul 28, 2024

jonesbusy commented Aug 2, 2024

jonesbusy commented Aug 18, 2024

bitwiseman commented Aug 18, 2024

jonesbusy commented Jul 17, 2024 •

edited

Loading

codecov bot commented Jul 17, 2024 •

edited

Loading