Fix documentation issues across site docs#2010
Conversation
Corrected grammatical errors and improved clarity in the explanation of checksum algorithms and their usage.
gnodet
left a comment
There was a problem hiding this comment.
Two confirmed issues in documentation changes: (1) the code example in creating-a-repository-system-session.md passes a SessionBuilder to newLocalRepositoryManager(), which expects a RepositorySystemSession — this will not compile; (2) minor indentation inconsistency in api-compatibility.md.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of gnodet
| RepositorySystemSession.SessionBuilder sessionBuilder = system.createSessionBuilder(); | ||
|
|
||
| LocalRepository localRepo = new LocalRepository( "target/local-repo" ); | ||
| sessionBuilder.setLocalRepositoryManager( system.newLocalRepositoryManager( session, localRepo ) ); | ||
| sessionBuilder.setLocalRepositoryManager( system.newLocalRepositoryManager( sessionBuilder, localRepo ) ); | ||
|
|
||
| return session.build(); | ||
| return sessionBuilder.build(); | ||
| } | ||
| ``` |
There was a problem hiding this comment.
The fix correctly replaces the undefined session variable and SessionBuilderSupplier, but system.newLocalRepositoryManager(sessionBuilder, localRepo) will not compile: newLocalRepositoryManager expects a RepositorySystemSession as its first argument, and SessionBuilder does not extend RepositorySystemSession.
The SessionBuilder API provides withLocalRepositories(LocalRepository...) for this purpose, which would make the example both simpler and correct:
| RepositorySystemSession.SessionBuilder sessionBuilder = system.createSessionBuilder(); | |
| LocalRepository localRepo = new LocalRepository( "target/local-repo" ); | |
| sessionBuilder.setLocalRepositoryManager( system.newLocalRepositoryManager( session, localRepo ) ); | |
| sessionBuilder.setLocalRepositoryManager( system.newLocalRepositoryManager( sessionBuilder, localRepo ) ); | |
| return session.build(); | |
| return sessionBuilder.build(); | |
| } | |
| ``` | |
| RepositorySystemSession.SessionBuilder sessionBuilder = system.createSessionBuilder(); | |
| LocalRepository localRepo = new LocalRepository( "target/local-repo" ); | |
| sessionBuilder.withLocalRepositories( localRepo ); | |
| return sessionBuilder.build(); |
Co-authored-by: Guillaume Nodet <gnodet@gmail.com>
Corrected phrasing and improved clarity in the text.
There was a problem hiding this comment.
Pull request overview
This PR updates the Maven Resolver site documentation to correct broken examples, improve clarity/grammar, and address an accessibility gap in the site index.
Changes:
- Fix and modernize documentation examples (notably session creation and shell snippets).
- Clarify version-compatibility wording and correct various grammar/typos across multiple pages.
- Improve site accessibility by adding missing image alternative text.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/site/xdoc/index.xml | Adds an alt attribute to the dependency diagram image for accessibility. |
| src/site/markdown/what-is-resolver.md | Removes a trailing stray “is” and fixes sentence punctuation. |
| src/site/markdown/upgrading-resolver.md | Improves wording/grammar (“its”, “except”, sentence clarity). |
| src/site/markdown/transporter-known-issues.md | Fixes missing preposition in explanatory text. |
| src/site/markdown/third-party-integrations.md | Fixes phrasing (“in our way”). |
| src/site/markdown/repository-key-function.md | Fixes missing possessive apostrophe. |
| src/site/markdown/remote-repository-filtering.md | Fixes punctuation/typos and improves explanatory sentence clarity. |
| src/site/markdown/local-repository.md | Corrects code block language tags/line continuations and fixes wording. |
| src/site/markdown/how-resolver-works.md | Fixes phrasing and removes a stray empty list item. |
| src/site/markdown/deployment.md | Fixes heading level and corrects connector reference text/link mismatch. |
| src/site/markdown/creating-a-repository-system-session.md | Updates the Java example for creating a RepositorySystemSession. |
| src/site/markdown/common-misconceptions.md | Improves wording and fixes minor grammar issues. |
| src/site/markdown/api-compatibility.md | Updates version policy wording and improves historical explanation text. |
| src/site/markdown/about-checksums.md | Improves wording/grammar and corrects phrasing around checksums. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| LocalRepository localRepo = new LocalRepository( "target/local-repo" ); | ||
| sessionBuilder.setLocalRepositoryManager( system.newLocalRepositoryManager( session, localRepo ) ); | ||
| sessionBuilder.setLocalRepositoryManager( system.newLocalRepositoryManager( sessionBuilder, localRepo ) ); | ||
|
|
||
| return session.build(); | ||
| return sessionBuilder.build(); |
| * On minor version change, we ENSURE backward compatibility for those "exposed" 3 modules: API, | ||
| SPI and Util. | ||
| SPI and Util. Still, there are examples when we failed to do so, usually driven by new | ||
| features. |
Corrected grammatical errors and improved clarity in the explanation of test and runtime graphs in Maven.
gnodet
left a comment
There was a problem hiding this comment.
Thanks for fixing the indentation issue in api-compatibility.md and the grammar fix in common-misconceptions.md!
The compile issue in creating-a-repository-system-session.md still remains — system.newLocalRepositoryManager(sessionBuilder, localRepo) won't compile because SessionBuilder doesn't extend RepositorySystemSession.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of gnodet
| RepositorySystemSession.SessionBuilder sessionBuilder = system.createSessionBuilder(); | ||
|
|
||
| LocalRepository localRepo = new LocalRepository( "target/local-repo" ); | ||
| sessionBuilder.setLocalRepositoryManager( system.newLocalRepositoryManager( session, localRepo ) ); | ||
| sessionBuilder.setLocalRepositoryManager( system.newLocalRepositoryManager( sessionBuilder, localRepo ) ); | ||
|
|
||
| return session.build(); | ||
| return sessionBuilder.build(); |
There was a problem hiding this comment.
This still won't compile: all three overloads of newLocalRepositoryManager require a RepositorySystemSession as the first argument, and SessionBuilder does not extend RepositorySystemSession.
The SessionBuilder API provides withLocalRepositories(LocalRepository...) for exactly this purpose:
| RepositorySystemSession.SessionBuilder sessionBuilder = system.createSessionBuilder(); | |
| LocalRepository localRepo = new LocalRepository( "target/local-repo" ); | |
| sessionBuilder.setLocalRepositoryManager( system.newLocalRepositoryManager( session, localRepo ) ); | |
| sessionBuilder.setLocalRepositoryManager( system.newLocalRepositoryManager( sessionBuilder, localRepo ) ); | |
| return session.build(); | |
| return sessionBuilder.build(); | |
| RepositorySystemSession.SessionBuilder sessionBuilder = system.createSessionBuilder(); | |
| LocalRepository localRepo = new LocalRepository( "target/local-repo" ); | |
| sessionBuilder.withLocalRepositories( localRepo ); | |
| return sessionBuilder.build(); |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
gnodet
left a comment
There was a problem hiding this comment.
Thanks for addressing the compile error! However, the fix switched to new DefaultRepositorySystemSession(), whose no-arg constructor is @Deprecated with a javadoc warning that it "leads to resource leaks" and should be replaced with RepositorySystem.createSessionBuilder(). A documentation example should demonstrate the recommended API.
Also, the surrounding prose still references maven-resolver-supplier classes and RepositorySystemSession.SessionBuilder, which the code no longer uses — creating an inconsistency between the text and the example.
The suggestion from the previous review would resolve both issues:
RepositorySystemSession.SessionBuilder sessionBuilder = system.createSessionBuilder();
LocalRepository localRepo = new LocalRepository( "target/local-repo" );
sessionBuilder.withLocalRepositories( localRepo );
return sessionBuilder.build();This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of gnodet
Clarified the explanation of Resolver's completeness and its dependency on the maven-resolver-provider module.
Updated language for clarity and precision regarding checksums and their security implications.
Clarified user instructions on disabling prefix discovery for broken Maven Repository Managers.
gnodet
left a comment
There was a problem hiding this comment.
New issues introduced in about-checksums.md — unmatched parentheses and a missing word. The previously raised concern about the deprecated DefaultRepositorySystemSession() constructor in creating-a-repository-system-session.md also remains.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of gnodet
Co-authored-by: Guillaume Nodet <gnodet@gmail.com>
Co-authored-by: Guillaume Nodet <gnodet@gmail.com>
Fixes multiple issues across 14 site documentation files: