felix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Thomas Watson (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (FELIX-4848) Split ResolverImpl
Date Fri, 29 May 2015 12:47:18 GMT

    [ https://issues.apache.org/jira/browse/FELIX-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14564724#comment-14564724

Thomas Watson commented on FELIX-4848:

The latest patch is incomplete, it is missing the ConsistencyCheck class.  I don't have a
strong opinion on the need for this patch, but if it helps others understand the code better
then it seems like it could be a good thing.  Not seeing the complete patch I do have some
concerns.  Did you really just move code from one class to another without making functional
changes?  Or did you fix some bugs along the way?  I ask because the changes to the Util class
may be problematic.  

For getSymbolicName and getVersion you now use the osgi.identity namespace to lookup the capability
with the info.  That is good, but at one point the WrappedResource did not pay attention to
the namespace param (see FELIX-4727).  I'm not saying the code should not use the namespace
here, but it does require that the input resources pay attention to the namespace (we certainly
do in Equinox, but I don't know about other users).  But the other issue is that the new impls
of these methods will throw out of bounds exceptions if the resource has no osgi.identity
capabilities (e.g. for R3 bundles).  Perhaps not important to most, but Equinox still supports
R3 bundles and I am pretty sure Felix does also.

Once the complete patch is posted it is going to be hard to confirm that all that was done
is a refactoring of the code, without changing the functionality and I worry about that based
on the changes done to the Util class.

> Split ResolverImpl
> ------------------
>                 Key: FELIX-4848
>                 URL: https://issues.apache.org/jira/browse/FELIX-4848
>             Project: Felix
>          Issue Type: Improvement
>          Components: Resolver
>    Affects Versions: resolver-1.0.0
>            Reporter: Christian Schneider
>             Fix For: resolver-1.4.0
>         Attachments: Dependencies in resolver package after patch.png, FELIX-4848-1.patch
> ResolverImpl currently contains about 2300 lines of code. That is way too big for a single
> I looked into it and found that the checkDynamicPackageSpaceConsistency and checkPackageSpaceConsistency
methods and their dependent methods form a nice subset. I would move that into a class ConsistencyCheck.
> Both would share all of the inner classes of ResolverImpl but nothing else. 
> So I think i would make sense to move these inner classes to separate files.
> These changes should nicely split the classes into 
> ResolverImpl : 1400 lines
> ConsistencyCheck : 600 lines

This message was sent by Atlassian JIRA

View raw message