Community
Participate
Working Groups
Build Identifier: Review is here: http://review.mylyn.org/#change,111 Adds a method to retrieve all IProject, that are contained in the currently active context. Reproducible: Always
Shawn, could you take a look if the proposed changes look good to you?
We need to have this information available on task deactivation to persist the active branches, right? One concern could be the coupling from the EGit Mylyn integration to Mylyn Context which we may want to avoid. I don't think it's a concern right now since some of the team components are still in Context anyways but we may need to revisit this in the future. We could enhance the notion of what a context is in Tasks (right now we only have a context store which is file based but agnostic to the actual content of the context). The first step could be an extension in Tasks that provides a mapping from task -> resources (projects) with an implementation in context that does that based on interaction history.
The basics of this patch seem right. My only concern is making context require o.e.c.resources as it is designed to not have any resource couplings. I wonder if this should be in the resource structure bridge somewhere and then only look for projects (i.e. look specifically at the handles to determine if it is a project or not) as for a large context this could be a long running operation as it iterates over all elements in the context and could cause java structure bridge loading.
So to determine, if a handle is a project, i would check if the handle contains only one path segment, like in ResourceStructureBridge#getObjectForHandle()? And if it contains more than one segment, it would be a resource, which i could ignore?
Created attachment 208797 [details] mylyn/context/zip
That sounds right to me.
I pushed a second draft to Gerrit (http://review.mylyn.org/#change,180,patchset=1), where i put the method in the ResourceStructureBridge, so that the context doesn't get dependencies to o.e.c.resources
Looks fine to me. We just need one or more test cases in order to apply to master.
I'm working on some test cases. I think i should be able to finish them at the end of the week.
Created attachment 208808 [details] mylyn/context/zip
Ok, i've added a small test just to check if there are no duplicate project entries in the result. If you need more tests, then just give me a hint. Right now, i don't have an idea, what else i could test.
Created attachment 208813 [details] mylyn/context/zip
Thanks for the updated change set! It'd be great if addressed the comment in the review and attached a patch so that we can merge it and track the contribution in the IP log.
Created attachment 209036 [details] Patch
Created attachment 209037 [details] mylyn/context/zip
I made some changes according to your comments (removed logging, since no checked exceptions are thrown, added checking, if we are adding real projects).
Looks good to me. Only two minor nits: * ResourceStructureBridgeTest should be added to the All*Tests suite. * Instead of using List<IProject> projectsInWorkspace = Arrays.asList(workspace.getRoot().getProjects()) could we invoke project.exists() before adding the project to the result collection?
Created attachment 209100 [details] Edited according to last comment from Steffen
Created attachment 209101 [details] mylyn/context/zip
Patch committed.
