Copilot commented on code in PR #2010:
URL: https://github.com/apache/maven-resolver/pull/2010#discussion_r3639417586
##########
src/site/markdown/creating-a-repository-system-session.md:
##########
@@ -35,12 +35,12 @@ import org.eclipse.aether.supplier.RepositorySystemSupplier;
...
private static RepositorySystemSession newSession( RepositorySystem system
)
{
- RepositorySystemSession.SessionBuilder sessionBuilder =
SessionBuilderSupplier.get();
+ 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();
Review Comment:
The snippet calls RepositorySystem#newLocalRepositoryManager with
`sessionBuilder`, but the API expects a `RepositorySystemSession`. Also,
`SessionBuilder#setLocalRepositoryManager` is documented as a "do not use"
method due to the chicken-and-egg problem. Use the
`SessionBuilder#withLocalRepositories(...)` shortcut instead to configure the
local repository in this example.
##########
src/site/markdown/api-compatibility.md:
##########
@@ -70,11 +70,9 @@ changes using version number. We use "major.minor.patch"
versioning on resolver
semantics:
* On major version change, one should NOT expect any backward compatibility.
-* On minor version change, we TRY to keep backward compatibility for those
"exposed" 3 modules:
- API, SPI and Util. Still, there are examples when we failed to do so,
usually driven by new
- features.
* 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.
Review Comment:
This bullet says the project "ENSURE[s]" backward compatibility on minor
version changes, but immediately notes cases where it failed. That’s internally
contradictory and makes the contract unclear. Consider softening the guarantee
(e.g., "try/aim") or removing the failure clause.
##########
src/site/markdown/common-misconceptions.md:
##########
@@ -81,8 +81,8 @@ a showcase how Resolver works.
## Misconception No2: "Test graph" Is Superset Of "Runtime graph"
**Wrong**. As can be seen from above, for runtime graph we leave out "test"
scoped
-dependencies. It was true in Maven2, where test graph really was superset of
runtime,
-this does not stand anymore in Maven3. And this have interesting consequences.
Let me show an example:
+dependencies. It was true in Maven2, where test graph really was a superset of
runtime,
+but this does not stand anymore in Maven3. And this have interesting
consequences. Let me show an example:
Review Comment:
Grammar: "this have" should be "this has".
##########
src/site/xdoc/index.xml:
##########
@@ -36,7 +36,7 @@
</p>
<p>
- <img src="images/maven-resolver-deps.png" width="792" height="441"
border="0" usemap="#Maven_Resolver_dependencies" />
+ <img src="images/maven-resolver-deps.png" width="792" height="441"
border="0" usemap="#Maven_Resolver_dependencies" alt="" />
Review Comment:
The dependency diagram image appears to convey information (and uses an
image map), so an empty alt text will hide it from screen readers. Provide a
short descriptive alt text instead of "".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
