gnodet commented on code in PR #2010:
URL: https://github.com/apache/maven-resolver/pull/2010#discussion_r3639383697
##########
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 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:
```suggestion
RepositorySystemSession.SessionBuilder sessionBuilder =
system.createSessionBuilder();
LocalRepository localRepo = new LocalRepository( "target/local-repo"
);
sessionBuilder.withLocalRepositories( localRepo );
return sessionBuilder.build();
```
##########
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.
+ * On minor version change, we ENSURE 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.
Review Comment:
Nit: this merged bullet has a leading space (` * On minor...`) while the
preceding bullet uses `* On major...` without one. The indentation
inconsistency could cause some Markdown renderers to treat this as a nested
sub-item.
```suggestion
* On minor version change, we ENSURE 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.
```
--
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]
