Bugzilla – Full Text Bug Listing
| Summary: | [api] merge local tasks with repository tasks | ||
|---|---|---|---|
| Product: | z_Archived | Reporter: | Mik Kersten <mik.kersten> |
| Component: | Mylyn | Assignee: | Robert Elves <robert.elves> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | enhancement | ||
| Priority: | P1 | CC: | brockj, mik.kersten, robert.elves, steffen.pingel, wes.coelho, wmitsuda |
| Version: | unspecified | ||
| Target Milestone: | 2.0 | ||
| Hardware: | PC | ||
| OS: | Windows All | ||
| Whiteboard: | |||
|
Description
Mik Kersten
Putting off until next week, too many other changes in the tasklist for 0.4.8. Currently there is no state held for a task that's purely local. The only state held is planning/reminder/note information, which applies to any repository task as well. So while we may still want to consider improving support for local tasks by adding a local repository, that issue is seperate. Marking for later since this may need to be reconsidered. Reopening based on recent discussion on mylar-dev. We have just updated the Task/AbstractRepositoryTask hierarchy in a way that facilitates this. This is the one remaining thorn in our architecture and we need to explore the changes needed to support this for 2.0. Rob: please investigate and let's figure out how necessary and how much of an improvement this would be. Mik, are you planning to remove ITask interface and pull up AbstractRepositoryTask class into Task? The latter is a good opportunity to try pull up refactoring. Yup, we are working that out right now! We would end up with a nice and simple AbstractTask. Any objections?? There are two separate architectural issues: 1) Merge AbstractRepositoryTask with Task, will be enable by changes to this bug report and the split has always meant more complicated client code. Might result in something like a LocalPlanningData object and an AbstractTask.getPlanningData() method. Or we will keep the common planning data as fields on AbstractTask since there aren't that many. 2) Get rid of ITask and rely on a clean interface of public/protected methods on AbstractTask. The main benefit of this is that we can add methods to AbstracTask without breaking clients, which has been a big benefit with AbstractRepositoryTask already. The cost is that many developers find plain interfaces easier to read/implement. (In reply to comment #6) > Yup, we are working that out right now! We would end up with a nice and simple > AbstractTask. Nice. I hope you'll make sure getTaskKey() is there. > Any objections?? There is one you wouldn't agree with. Make that AbstractTask final. :-) > 1) Merge AbstractRepositoryTask with Task, will be enable by changes to this bug > report and the split has always meant more complicated client code. Might > result in something like a LocalPlanningData object and an > AbstractTask.getPlanningData() method. Or we will keep the common planning data > as fields on AbstractTask since there aren't that many. It would have been easier if AbstractTask had support for arbitrary properties. Though it is my legacy rdbms background speaking... > 2) Get rid of ITask and rely on a clean interface of public/protected methods on > AbstractTask. The main benefit of this is that we can add methods to > AbstracTask without breaking clients, which has been a big benefit with > AbstractRepositoryTask already. The cost is that many developers find plain > interfaces easier to read/implement. You already know my answer to that. Make something that don't need to be extended. :-) (In reply to comment #6) > 2) Get rid of ITask and rely on a clean interface of public/protected methods > on AbstractTask. The main benefit of this is that we can add methods to > AbstracTask without breaking clients, which has been a big benefit with > AbstractRepositoryTask already. The cost is that many developers find plain > interfaces easier to read/implement. I agree with that but since ITask is such a huge interface it is unlikely for anyone to implement it without subclassing the abstract class anyways. It does not have any documentation so the only way to figure out what it is supposed to do is to read existing implementations. Therefore I am in favor of using an abstract class only unless we are positive that the ITask API is rock stable and it doesn't hurt to keep it around. Committer partial progress... Rob: please get things compiling asap... Head now compiles. Work ongoing... Task is gone, ITask and ITaskListElement still standing. Those pertinent unit tests now passing. (In reply to comment #11) > Task is gone, ITask and ITaskListElement still standing. Hmm. I thought the idea was to replace interface with the abstract class... Yup thats the plan. Once we're done here (tonight or morning) we'll be left with an AbstractTask. Taking this over for the remaining changes... Committed merge of ITask and ITaskListElement into hierarchy. Still a bunch of API clean-up needed. Something wen't really funny - org.eclipse.mylyn.tasks.core.getAllCategories Yes, thanks for pointing that out. Fix coming shortly along with additional clean-up. Committed next round of changes, still some consistency needed. Eugene: since we have a patch outstanding and awaiting review on bug 191575 I'm not sure if you want to synch or not. We could try to merge it into an old workspace. Don't wait up on me. Besides, that patch is only needed for Steffen and Rob mostly as a hint. I might be able to merge it once more... Mik, I have Eugene's patch applied to my workspace and will take care of the merging. Excellent! Thanks Steffen. Please go ahead and commit as soon as you can so that Rob and I can wrap up the refactoring. We still need to clean up confusino between AbstractTaskListElement/AbstractTaskContainer/TaskCategory. Mik, not sure I understand your comment correctly. It will take at least until tomorrow until the patch will get to a level suited for review. There is no need to hold up any refactorings or commits. Sorry, I misunderstood the timing. We'll continue on and you can merge when ready. Done. Still UI stuff and test fixes to do on related bugs. Is it possible to hide the "Local Tasks" from the repositories view? All of its context menu items are disabled. We have the notion of user managed connectors, perhaps non-user managed TaskRepositories (i.e. Local Tasks) should be hidden. This may be more detrimental than good due to lack of visibility and inconsistency with New Task dialog (where it needs to show up). Thoughts? I agree that the inconsistency with the New Task dialog might be confusing but I find it even more confusing to have a repository that can neither be deleted nor edited (renamed). I'd say it's less confusing to hide it from the view. If there is a good reason to keep we could also add a filter. I agree that the current state is weird. However, I think that it is very likely that we will want to provide some user-defined properties for the local repository (e.g., password-based encryption in case users are storing passwords in notes). Also, the Task Repositories view is likely to be a place where users would go when wondering "where on earth does this thing store my (local) tasks?". So for now I propose that we enable the Properties page on that repository, and on that page simply state that it is automatically created and point to the Task List preference page which points the user at the data directory. If there are no objections I'll add this to the ui nits. If we keep the repository visible in the Task Repositories view for 2.0 I think that Properties has to be clickable for the reasons that Steffen outlines. But it doesn't need to happen for RC1. |