maven-resolver
maven-resolver copied to clipboard
Null implementations shall return null
@cstamas This breaks stuff and should be in 2.x only. The motivation is that if the class name says Null... it shall return null and not be a Default... impl. WDYT?
Null impl does not mean that a method returns null. It means that the class and method does nothing - no behavior.
Dňa ne 20. 6. 2021, 12:55 Michael Osipov @.***> napísal(a):
@cstamas https://github.com/cstamas This breaks stuff and should be in 2.x only. The motivation is that if the class name says Null... it shall return null and not be a Default... impl. WDYT?
You can view, comment on, or merge this pull request online at:
https://github.com/apache/maven-resolver/pull/113 Commit Summary
- Null implementations shall return null
File Changes
- M maven-resolver-api/src/main/java/org/eclipse/aether/DefaultRepositorySystemSession.java https://github.com/apache/maven-resolver/pull/113/files#diff-f1c0b545239dff473550ca53cab44bf3054c7e7ce915b9ad40ef5b865a165426 (4)
Patch Links:
- https://github.com/apache/maven-resolver/pull/113.patch
- https://github.com/apache/maven-resolver/pull/113.diff
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/apache/maven-resolver/pull/113, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAH7ER7CPXFIWKXFG3DSWH3TTXCLLANCNFSM4676ZRHA .
Null impl does not mean that a method returns null. It means that the class and method does nothing - no behavior.
I don't understand that. How can be no behaivor if it invokes a getter?
Every Null...Class means no behavior. The generál terminology. Also nongetter methods.
Dňa ne 20. 6. 2021, 19:10 Michael Osipov @.***> napísal(a):
Null impl does not mean that a method returns null. It means that the class and method does nothing - no behavior.
I don't understand that. How can be no behaivor if it invokes a getter?
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/apache/maven-resolver/pull/113#issuecomment-864584498, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAH7ER4JTKJIEEPV3UEDOEDTTYOJLANCNFSM4676ZRHA .
@cstamas What is your opinion, does this make sense?
I think the classes are named badly. These "default" (and not "null") selectors are meant to not interfere with already equipped repositories.
In short, you know that when you have a RemoteRepository instance created with RemoteRepository.Builder, it is "bare", in a sense, it carries ONLY things set by builder: id, type, url (and policies but that is irrelevant). There is NO AUTH/PROXY, anything "networking" related set on it. To "equip" RR instance with these, one must call one of these (use case dependant):
- https://maven.apache.org/resolver/apidocs/org/eclipse/aether/RepositorySystem.html#newDeploymentRepository(org.eclipse.aether.RepositorySystemSession,org.eclipse.aether.repository.RemoteRepository)
- https://maven.apache.org/resolver/apidocs/org/eclipse/aether/RepositorySystem.html#newResolutionRepositories(org.eclipse.aether.RepositorySystemSession,java.util.List)
And this method will return "equipped" RR instance (Moreover, when this happens within Maven, how Resolver is integrated with Maven, this means it will pick up things like auth and proxies etc from Maven settings.xml and related stuff).
Now, these (default) instances are "null" in a way, they do not obtain any info from anywhere, but from repository itself: if you have a "bare" RR, it will end up with "null", but IF you have an "equipped" RR instance, it will return what equipped RR has.
Hence, IMHO the names of these classes are bad, they are not "null" but rather "passthrough" or alike.
Would you rename them for 2.0.0? That feels natural...
Created https://issues.apache.org/jira/browse/MRESOLVER-565
Superseded by https://github.com/apache/maven-resolver/pull/503