Skip to content

Improve JavaDoc in GitScmProviderRepository by replacing TODO placeholders #1352

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.

Sign up for GitHub

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

Open
anukalp2804 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Improve JavaDoc in GitScmProviderRepository by replacing TODO placeholders #1352

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
962789e
Document deprecation rationale for executeChangeLogCommand overloads
anukalp2804 Dec 25, 2025
2dbacaa
Improve JavaDoc in GitScmProviderRepository
anukalp2804 Dec 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions ...cm-api/src/main/java/org/apache/maven/scm/command/changelog/AbstractChangeLogCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,13 @@
* @author Olivier Lamy
*/
public abstract class AbstractChangeLogCommand extends AbstractCommand implements ChangeLogCommand {
/**
* @deprecated This method is part of the legacy changelog command execution
* mechanism and is retained for backward compatibility with existing SCM
* provider implementations. Some providers (for example Git) still implement
* this API, but newer code should rely on the current changelog handling
* logic provided by the SCM command framework instead.
*/
@Deprecated
protected abstract ChangeLogScmResult executeChangeLogCommand(
ScmProviderRepository repository,
Expand All @@ -45,6 +52,12 @@ protected abstract ChangeLogScmResult executeChangeLogCommand(
String datePattern)
throws ScmException;

/**
* @deprecated This legacy overload is kept for backward compatibility.
* It may not be supported by all SCM providers and in many cases
* results in an {@link ScmException}. Providers that still rely on it
* should consider migrating to the newer changelog execution APIs.
*/
@Deprecated
protected ChangeLogScmResult executeChangeLogCommand(
ScmProviderRepository repository,
Expand All @@ -56,6 +69,12 @@ protected ChangeLogScmResult executeChangeLogCommand(
throw new ScmException("Unsupported method for this provider.");
}

/**
* @deprecated This legacy overload is kept for backward compatibility.
* It may not be supported by all SCM providers and in many cases
* results in an {@link ScmException}. Providers that still rely on it
* should consider migrating to the newer changelog execution APIs.
*/
@Deprecated
protected ChangeLogScmResult executeChangeLogCommand(
ScmProviderRepository repository, ScmFileSet fileSet, ScmVersion version, String datePattern)
Expand Down
17 changes: 13 additions & 4 deletions .../src/main/java/org/apache/maven/scm/provider/git/repository/GitScmProviderRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,11 @@ public class GitScmProviderRepository extends ScmProviderRepositoryWithHost {
public static final String PROTOCOL_HTTPS = "https";

/**
* Use rsync for retrieving the data
* TODO implement!
* Protocol identifier for repositories accessed via <code>rsync</code>.
*
* This value is used when parsing SCM URLs and represents a repository
* that is retrieved using the rsync protocol. Currently, rsync is
* recognized but not actively implemented by the Git SCM provider.
Comment on lines +80 to +82
Copy link

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

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

The documentation states "Currently, rsync is recognized but not actively implemented by the Git SCM provider," which is misleading. Looking at the code, PROTOCOL_RSYNC is actually parsed from URLs (line 321-322), included in urlSupportsUserInformation (line 246), and used to construct repository URLs. While there may not be special rsync-specific handling, the protocol is functionally supported like other protocols. The documentation should clarify what "not actively implemented" means, or if rsync is actually fully supported, the documentation should reflect that accurately.

Suggested change
* This value is used when parsing SCM URLs and represents a repository
* that is retrieved using the rsync protocol. Currently, rsync is
* recognized but not actively implemented by the Git SCM provider.
* This value is used when parsing SCM URLs and constructing repository
* URLs that use the rsync protocol. The Git SCM provider does not apply
* any rsync-specific behavior; rsync-based URLs are handled in the same
* generic way as other supported protocols.

Copilot uses AI. Check for mistakes.
*/
public static final String PROTOCOL_RSYNC = "rsync";

Expand Down Expand Up @@ -226,10 +229,16 @@ private RepositoryUrl parseUrl(String url) throws ScmException {
}

/**
* @param repoUrl
* @return TODO
* Builds the effective Git repository URL from the parsed SCM URL
* definition.
*
* @param repoUrl parsed repository descriptor containing host,
* protocol, and path information
* @return the normalized remote repository URL that Maven SCM will
* use for Git operations
*/
private String getUrl(RepositoryUrl repoUrl) {

Copy link

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

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

An unnecessary blank line has been added within the method body. This deviates from the surrounding code style where the method body starts immediately after the opening brace. The blank line should be removed to maintain consistency.

Suggested change

Copilot uses AI. Check for mistakes.
StringBuilder urlSb = new StringBuilder(repoUrl.getProtocol());
boolean urlSupportsUserInformation = false;

Expand Down
Loading