src/main/java/org/kohsuke/github/GHRepository.java (2)

1462-1464: Add @deprecated Javadoc tag to deprecated method

The deprecated method fork() should include an @deprecated tag in its Javadoc comment, indicating the preferred replacement method createFork. This helps users understand the deprecation and transition smoothly.

---

1494-1497: Add @deprecated Javadoc tag to deprecated method

The deprecated method forkTo(GHOrganization org) should include an @deprecated tag in its Javadoc comment, specifying that createFork is the recommended replacement. This provides clearer guidance to users.

src/test/java/org/kohsuke/github/GHRepositoryTest.java (1)

847-851: Consider verifying the exception message in the test

In testCreateForkWithInvalidParameters(), consider asserting the exception message to ensure the IllegalArgumentException is thrown for the correct reason.

Configuration used: CodeRabbit UI
Review profile: CHILL

src/main/java/org/kohsuke/github/GHRepository.java (3)

1501-1534: Consider enhancing input validation

While the current validation catches empty strings, consider adding these improvements:

1. Validate that name follows GitHub repository naming conventions
2. Add @nullable annotations for optional parameters
3. Add @nonnull for required parameters

-    public GHRepository createFork(String organization, String name, boolean defaultBranchOnly) throws IOException {
+    public GHRepository createFork(@Nullable String organization, @Nullable String name, boolean defaultBranchOnly) throws IOException {
         if (organization != null && organization.isEmpty()) {
             throw new IllegalArgumentException("Organization cannot be empty");
         }
         if (name != null && name.isEmpty()) {
             throw new IllegalArgumentException("Name cannot be empty");
         }
+        if (name != null && !name.matches("^[a-zA-Z0-9._-]+$")) {
+            throw new IllegalArgumentException("Repository name contains invalid characters");
+        }

---

Line range hint 1538-1550: Consider enhancing the polling mechanism

The current implementation uses fixed values for polling. Consider these improvements:

1. Make retry count and delay configurable
2. Implement exponential backoff
3. Add more detailed error messages
4. Consider adding a timeout parameter

+    private static final int DEFAULT_FORK_TIMEOUT_SECONDS = 30;
+    private static final int DEFAULT_FORK_INITIAL_DELAY_MS = 1000;
+
     public GHRepository createFork(String organization, String name, boolean defaultBranchOnly) throws IOException {
         // ... existing code ...
+        return createFork(organization, name, defaultBranchOnly, DEFAULT_FORK_TIMEOUT_SECONDS);
+    }
+
+    public GHRepository createFork(String organization, String name, boolean defaultBranchOnly, int timeoutSeconds) throws IOException {
         // ... existing request code ...
 
         // this API is asynchronous. we need to wait for a bit
-        for (int i = 0; i < 10; i++) {
+        long startTime = System.currentTimeMillis();
+        long timeoutMs = timeoutSeconds * 1000L;
+        int delayMs = DEFAULT_FORK_INITIAL_DELAY_MS;
+
+        while (System.currentTimeMillis() - startTime < timeoutMs) {
             GHRepository r = organization != null
                     ? root().getOrganization(organization).getRepository(name != null ? name : this.name)
                     : root().getMyself().getRepository(name != null ? name : this.name);
             if (r != null) {
                 return r;
             }
             try {
-                Thread.sleep(3000);
+                Thread.sleep(delayMs);
+                delayMs = Math.min(delayMs * 2, 10000); // Exponential backoff capped at 10 seconds
             } catch (InterruptedException e) {
                 throw (IOException) new InterruptedIOException().initCause(e);
             }
         }
-        throw new IOException(this + " was forked but can't find the new repository");
+        throw new IOException(String.format("Repository %s was forked but the new repository was not available after %d seconds", this, timeoutSeconds));

---

Line range hint 1501-1550: Implementation aligns well with GitHub's asynchronous patterns

The implementation correctly handles GitHub's asynchronous fork creation process and provides good extensibility for future enhancements. The separation of concerns between the synchronous API call and asynchronous polling is clean.

Consider these architectural improvements for future iterations:

1. Add events/callbacks for fork progress monitoring
2. Consider implementing a more generic async operation handler that could be reused for other async operations
3. Add metrics collection for fork operation timing

Configuration used: CodeRabbit UI
Review profile: CHILL

src/main/java/org/kohsuke/github/GHRepository.java (2)

1501-1533: LGTM with a suggestion for null safety

The implementation looks good and aligns with the PR objectives. The input validation is thorough for empty strings and invalid characters.

Consider adding @Nullable annotations to the parameters and using Objects.requireNonNull for required parameters:

     public GHRepository createFork(@Nullable String organization, @Nullable String name, boolean defaultBranchOnly) throws IOException {
+        // organization and name are already marked as @Nullable in the method signature
+        
         if (organization != null && organization.isEmpty()) {
             throw new IllegalArgumentException("Organization cannot be empty");
         }
         if (name != null && name.isEmpty()) {
             throw new IllegalArgumentException("Name cannot be empty");
         }

---

Line range hint 1537-1549: Consider making retry parameters configurable

The polling mechanism uses hard-coded values for retry count (10) and delay (3000ms). Consider making these configurable to handle different network conditions and server response times.

+    private static final int DEFAULT_FORK_RETRY_COUNT = 10;
+    private static final long DEFAULT_FORK_RETRY_DELAY_MS = 3000;
+
     public GHRepository createFork(@Nullable String organization, @Nullable String name, boolean defaultBranchOnly) throws IOException {
         // ... existing code ...
         
-        for (int i = 0; i < 10; i++) {
+        for (int i = 0; i < DEFAULT_FORK_RETRY_COUNT; i++) {
             GHRepository r = organization != null
                     ? root().getOrganization(organization).getRepository(name != null ? name : this.name)
                     : root().getMyself().getRepository(name != null ? name : this.name);
             if (r != null) {
                 return r;
             }
             try {
-                Thread.sleep(3000);
+                Thread.sleep(DEFAULT_FORK_RETRY_DELAY_MS);
             } catch (InterruptedException e) {
                 throw (IOException) new InterruptedIOException().initCause(e);
             }
         }
-        throw new IOException(this + " was forked but can't find the new repository");
+        throw new IOException(String.format("Repository %s was forked but couldn't be found after %d attempts",
+                this, DEFAULT_FORK_RETRY_COUNT));

Configuration used: CodeRabbit UI
Review profile: CHILL

src/main/java/org/kohsuke/github/GHRepository.java (3)

1501-1513: Enhance method documentation

The Javadoc should explain the asynchronous nature of the API and the polling behavior.

     /**
      * Creates a fork of this repository with optional parameters.
      *
      * @param organization
      *            the organization to fork to, or null to fork to the authenticated user's account
      * @param name
      *            the name of the new repository, or null to use the same name as the original repository
      * @param defaultBranchOnly
      *            whether to fork only the default branch
      * @return the newly forked repository
      * @throws IOException
      *             if an I/O error occurs
+     * @throws InterruptedIOException
+     *             if the polling is interrupted
+     * @implNote This method uses GitHub's asynchronous fork API. After initiating the fork,
+     *           it polls for up to 30 seconds (10 attempts, 3 seconds apart) for the fork to complete.
      */

---

Line range hint 1538-1554: Consider making polling parameters configurable

The polling interval (3 seconds) and maximum attempts (10) are hardcoded. Consider making these configurable or at least constants.

+    private static final int FORK_POLL_INTERVAL_MS = 3000;
+    private static final int FORK_POLL_MAX_ATTEMPTS = 10;
+
     requester.send();

     // this API is asynchronous. we need to wait for a bit
-    for (int i = 0; i < 10; i++) {
+    for (int i = 0; i < FORK_POLL_MAX_ATTEMPTS; i++) {
         GHRepository r = organization != null
                 ? root().getOrganization(organization).getRepository(name != null ? name : this.name)
                 : root().getMyself().getRepository(name != null ? name : this.name);
         if (r != null) {
             return r;
         }
         try {
-            Thread.sleep(3000);
+            Thread.sleep(FORK_POLL_INTERVAL_MS);
         } catch (InterruptedException e) {
             throw (IOException) new InterruptedIOException().initCause(e);
         }
     }
-    throw new IOException(this + " was forked but can't find the new repository");
+    throw new IOException(String.format("Repository %s was forked but the new repository was not found after %d seconds",
+            this, (FORK_POLL_INTERVAL_MS * FORK_POLL_MAX_ATTEMPTS) / 1000));

---

1517-1525: Improve input validation

The repository name validation regex could be a constant and error messages could be more descriptive.

+    private static final String VALID_REPOSITORY_NAME_PATTERN = "^[a-zA-Z0-9._-]+$";
+
     if (organization != null && organization.isEmpty()) {
-        throw new IllegalArgumentException("Organization cannot be empty");
+        throw new IllegalArgumentException("Organization name cannot be empty when specified");
     }
     if (name != null && name.isEmpty()) {
-        throw new IllegalArgumentException("Name cannot be empty");
+        throw new IllegalArgumentException("Repository name cannot be empty when specified");
     }
-    if (name != null && !name.matches("^[a-zA-Z0-9._-]+$")) {
-        throw new IllegalArgumentException("Repository name contains invalid characters");
+    if (name != null && !name.matches(VALID_REPOSITORY_NAME_PATTERN)) {
+        throw new IllegalArgumentException(
+            "Repository name can only contain letters, numbers, dots, hyphens and underscores");
     }

Configuration used: CodeRabbit UI
Review profile: CHILL

-    @Deprecated
+    @Deprecated(since = "3.0.0", forRemoval = true)
+    /** @deprecated Use {@link #createFork(String, String, boolean)} instead */
     public GHRepository forkTo(GHOrganization org) throws IOException {
         return createFork(org.getLogin(), null, false);
     }

-    @Deprecated
+    @Deprecated(since = "3.0.0", forRemoval = true)
+    /** @deprecated Use {@link #createFork(String, String, boolean)} instead */
     public GHRepository fork() throws IOException {
         return createFork(null, null, false);
     }

src/main/java/org/kohsuke/github/GHRepository.java (2)

Line range hint 1534-1555: Consider making the retry mechanism configurable

The current implementation uses hardcoded values for retry attempts (10) and delay (3000ms). Consider making these configurable to allow users to adjust based on their needs.

Consider adding configuration parameters:

     public GHRepository createFork(@Nullable String organization, 
         @Nullable String name, 
-        boolean defaultBranchOnly)
+        boolean defaultBranchOnly,
+        @Nullable Integer maxRetries,
+        @Nullable Integer retryDelayMillis)
         throws IOException {
         // ... existing validation and request code ...

+        final int actualMaxRetries = maxRetries != null ? maxRetries : 10;
+        final int actualRetryDelay = retryDelayMillis != null ? retryDelayMillis : 3000;
 
-        for (int i = 0; i < 10; i++) {
+        for (int i = 0; i < actualMaxRetries; i++) {
             // ... existing retry code ...
             try {
-                Thread.sleep(3000);
+                Thread.sleep(actualRetryDelay);
             } catch (InterruptedException e) {
                 throw (IOException) new InterruptedIOException().initCause(e);
             }
         }
-        throw new IOException(this + " was forked but can't find the new repository");
+        throw new IOException(String.format(
+            "Repository %s was forked to %s/%s but the fork could not be found after %d attempts",
+            this, organization, name, actualMaxRetries));
     }

+    public GHRepository createFork(@Nullable String organization, 
+        @Nullable String name, 
+        boolean defaultBranchOnly) throws IOException {
+        return createFork(organization, name, defaultBranchOnly, null, null);
+    }

---

1517-1525: Enhance input validation

The current validation could be improved with:

1. A more comprehensive repository name validation regex
2. More descriptive error messages including validation rules

         if (organization != null && organization.isEmpty()) {
-            throw new IllegalArgumentException("Organization cannot be empty. Pass null for default value.");
+            throw new IllegalArgumentException("Organization name cannot be empty. Pass null to fork to the authenticated user's account.");
         }
         if (name != null && name.isEmpty()) {
-            throw new IllegalArgumentException("Name cannot be empty. Pass null for default value.");
+            throw new IllegalArgumentException("Repository name cannot be empty. Pass null to use the same name as the original repository.");
         }
-        if (name != null && !name.matches("^[a-zA-Z0-9._-]+$")) {
-            throw new IllegalArgumentException("Repository name contains invalid characters");
+        if (name != null && !name.matches("^[a-zA-Z0-9][a-zA-Z0-9._-]*$")) {
+            throw new IllegalArgumentException(
+                "Repository name must start with a letter or number and can only contain letters, numbers, periods, underscores, and dashes");
         }

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro