On 11.05.2016 15:52, Jean-Baptiste Onofré wrote:
Hi Christian,
About 1/, the spec classes will be embedded in the resolver bundle
anyway ?
It is not about removing the embedding. This is fine. It is more about
keeping the spec sources in one place. If we host a copy it is difficult
to make sure they
are unchanged.
Thomas mentioned that he might need the sources for the upcoming version
7 spec to work with it before it is released.
I think this could be solved by publishing the spec as a snapshot which
might make sense for others too.
+1 to upgrade to Java 6.
+0 on removing the generic Util class.
+0 about ResolveSession and move Blame, PermutationType, etc there.
I'm not sure it will actually reduce the "global" weight (just moving
code from ResolverImpl to ResolveSession.
I think the bigger gain is from extracting ConstrainChecker. I
experimented with it and it moves about 1000 lines out of ResolverImpl
with just a single entry point of the method checkConsistency.
ContraintChecker then is the only class that refers to ResolveSession.
So I think in combination it makes more sense. We then have:
ResolverImpl 560 lines
ResolveSession 400 lines
ConsistencyChecker 1000 lines
So this keeps each class kind of manageable.
Regards
JB
On 05/11/2016 09:50 AM, Christian Schneider wrote:
Thomas asked me to look into the current code in resolver after his
improvements.
I found some spots that could be improved:
- We currently have a copy of the resolver spec sources inside the code.
I propose to refer to the spec bundle instead and embed the classes from
there. This protects against accidental changes in the spec sources. The
only caveat is that there is a difference in the bugfix version of the
spec but I think this is not a problem.
<dependency>
<groupId>org.osgi</groupId>
<artifactId>org.osgi.service.resolver</artifactId>
<version>1.0.1</version>
</dependency>
- I get some errors inside eclipse in CopyOnWriteListIterator. The
reason is that it uses @Override on methods from an interface while the
source level is at Java 5 which does not support this. So we can either
use java 6 or remove the @Override annotations there. I propose to
upgrade to Java 6.
- Avoid the generic Util class. We can split it into ResourceUtil and
RequirementUtil which would both host about half of the methods.
- Extract Consistency checks from ResolverImpl into a class
ConsistencyChecker like proposed in
https://issues.apache.org/jira/browse/FELIX-4848 . I have to validate
that this still works with the new code but I think it should.
- Extract ResolveSession and move Blame, PermutationType and UsedBlames
inside ResolveSession. This takes a bit of weight out of ResolverImpl
and also makes the cycle between ResolveSession and Capabilities smaller
thought it does not fully remove it
I first would like to get some general feedback about the proposed
changes. I will then create jiras and pull requests for the ones that
the community is positve about.
Christian
--
Christian Schneider
http://www.liquid-reality.de
Open Source Architect
http://www.talend.com
