| Summary: | support starring of comments | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | z_Archived | Reporter: | Mik Kersten <mik.kersten> | ||||||||
| Component: | Mylyn | Assignee: | Project Inbox <mylyn-triaged> | ||||||||
| Status: | CLOSED MOVED | QA Contact: | |||||||||
| Severity: | enhancement | ||||||||||
| Priority: | P3 | CC: | lucas.panjer, mik.kersten, peter, sam.davis, steffen.pingel, tomasz.zarna | ||||||||
| Version: | unspecified | Keywords: | plan | ||||||||
| Target Milestone: | --- | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | All | ||||||||||
| Whiteboard: | effort=3; | ||||||||||
| Bug Depends on: | 358554 | ||||||||||
| Bug Blocks: | 168363, 343600 | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Mik Kersten
How and where would the information about staring be persisted? It seems like user-specific information to me, since something that's interesting to me may not be interesting to you. Should it be kept in the Task Data? Task data is managed by the connectors and contains repository information and we don't have a good way to add other local information at the moment. It seems that staring is a connector independent framework concern similar to the personal notes. We could consider adding a memento like facility to hold addtional data for tasks that is stored somewhere in the .mylyn folder. FogBugz repository has concept of starring the case. It would be nice if "starred task" information can be stored in/retrieved from the repository too. (Unfortunately FogBugz exposes no API for this feature at the moment :-(, but may do so in the future). Need to move to backlog unless someone wants to contribute. Raising priority due to the high value and low effort nature of this. Steffen: I assume this can't make 3.5 so adding to 3.6 plan. Yes, unfortunately we are already overloaded for 3.5 so 3.6 would be the earliest to do this. *** Bug 353263 has been marked as a duplicate of this bug. *** I would like the starred state to be stored on the repository and shared, so that if one person stars a comment, everyone sees it. I suppose there might also be value in having private stars but having both would probably be too complicated. But I think generally if a comment is important, it is important to everyone. From Bug 353263: bq. It would be great to have some way of starring important comments on a task. This state would be stored in the repository so that it is shared between all users. As an example use case, sometimes someone will post a comment saying something like "I'll leave this open for now in case there is any more feedback, but if not then we are done here." When that person (or someone else) later returns to that task, it would be helpful to have that comment stand out somehow so that they know they don't have to read any earlier comments to determine the state of the task. Perhaps this could be supported by setting some metadata on the task attribute. I guess this specific use case could be addressed by using the status whiteboard in Bugzilla, but then I would want it to be in a more primary location. Using comments just seems more natural anyway. Sam: I can see the use cases for that feature. However, if comment starring is stored in the repository, I think it needs to be be supported by that repository. Otherwise what we're doing is layering on features that feel like they are part of the repository's data model, whereas they are not. In contrast, we have a very clear set of cross-repository features that are private and specific to the user, such as read state, scheduling, and drafting comments. This feature request is intended to add to that. Just as I want to be able to flag an email as important, I want to be able to flag a comment as important. Could you raise a separate request for supporting repository-stored starring and CC me? Mik, I did (bug 353263), but it was marked as a duplicate of this. Created attachment 203718 [details]
quick and dirty implementation of private starred comments
This is suitable for individual use (I did it because I am going to use it myself). Patch can be applied to workspace root by setting "ignore leading path segments" to 1.
* Needs images for starred and unstarred (currently uses checkboxes).
* Behaviour of Expand Starred could possibly be refined.
Created attachment 203719 [details]
mylyn/context/zip
Created attachment 203720 [details]
patch for master
The previous patch was for the service branch, here's one that works for master.
This looks like a good approach. I am wondering though if we need to keep the information about starred comments in memory or if it would be sufficient to store this on disk and load it when the task editor is opened? Sam, this is a great feature addition. Would you be willing to encapsulate management of starred state in a class that is not directly coupled to the editor implementation add add a test case? Feel free to push changes to http://review.mylyn.org/ if you want to move this forward through a code review. (In reply to comment #16) > This looks like a good approach. I am wondering though if we need to keep the > information about starred comments in memory or if it would be sufficient to > store this on disk and load it when the task editor is opened? Given that the user can modify the state at any time while the editor is opened, it seems like an odd thing to store on disk. Actually, I am storing it in the task data - I don't know if that's the right thing to do, but isn't it stored on disk when the task is not opened? > Sam, this is a great feature addition. Would you be willing to encapsulate > management of starred state in a class that is not directly coupled to the > editor implementation add add a test case? Feel free to push changes to > http://review.mylyn.org/ if you want to move this forward through a code review. I will see if I get time to do that. I may need some help or documentation about how to push changes there. (In reply to comment #17) > (In reply to comment #16) > > This looks like a good approach. I am wondering though if we need to keep the > > information about starred comments in memory or if it would be sufficient to > > store this on disk and load it when the task editor is opened? > > Given that the user can modify the state at any time while the editor is opened, > it seems like an odd thing to store on disk. Actually, I am storing it in the > task data - I don't know if that's the right thing to do, but isn't it stored on > disk when the task is not opened? Sorry if I didn't explain myself well. Looking at the code it seems that the state gets stored in the task list which means its kept in memory all the time: getTaskEditorPage().getTask().setAttribute(COMMENT_STARRED + taskComment.getNumber(), Boolean.toString(starred)) I was just wondering if it makes sense to persist it to a separate file that is only loaded into memory when the editor is opened. We have the same problem with notes, task activity and other data and this would be a good driver to create a generic facility to persist task related data that is not required to present the task list. Oh, I didn't even realise that was storing it in the task list. I think having a separate store would be a good idea, but it seems like a separate task. Would we want anything more than just (String, String) key-value pairs? I suppose anything that exports the task list might need to be updated to also export this new store. *** Bug 363984 has been marked as a duplicate of this bug. *** I have created a review at http://review.mylyn.org/181 It would be interesting to have some way of saying things like, "If you read comments 12-13, you can probably ignore comments 5-11," i.e. some way of indicating that a comment or comments summarize the discussion of some previous comments. That could make it much easier to get up to speed on tasks with a large number of comments. It would be sort of like post facto comment threading. Seems like fancy hyperlinking could do the trick for that, eg, comments 12-13 would be hyperlinked and cause the expansion state of that comment range to toggle. For now I would just be happy with starring :) What is the status of this? I see that the review has been abandoned. It has just been moved to Eclipse.org: https://git.eclipse.org/r/#/c/5634/. Steffen: Are you pushing ahead with this? Let me know if we should do a paired session on it. The bug has a high priority on the backlog but we are not considering it for 3.8 due to time constraints. Mylyn has been restructured, and our issue tracking has moved to GitHub [1]. We are closing ~14K Bugzilla issues to give the new team a fresh start. If you feel that this issue is still relevant, please create a new one on GitHub. [1] https://github.com/orgs/eclipse-mylyn |