Bugzilla – Full Text Bug Listing
| Summary: | make time tracking optional and opt-in | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 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: | shawn.minto, steffen.pingel, tomasz.zarna | ||||||||||||||||||||||
| Version: | unspecified | Keywords: | plan | ||||||||||||||||||||||
| Target Milestone: | 3.3 | ||||||||||||||||||||||||
| Hardware: | PC | ||||||||||||||||||||||||
| OS: | Windows 7 | ||||||||||||||||||||||||
| Whiteboard: | |||||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||||
|
Description
Mik Kersten
This seems like a significant change. All existing users would be pretty surprised to no longer have their time tracked per task activation. Should task activation now be accompanied by a PONT dialog suggesting they enable time tracking of active task? That seems overly disruptive. We can instead put a hyperlink in the private section that says "Enable Time Tracking". The preference page entry should clearly document that this is only time tracked when active in Eclipse. We'll also need to document this very clearly in the New & Noteworthy, stating the reasons (eg, that there are legal concerns of tracking time in some countries like Germany so we decided to give the developer more control). Created attachment 149078 [details]
first pass
Note that this has the potential to clear past interaction history since history date externalization is disabled by default.
Created attachment 149079 [details]
mylyn/context/zip
Created attachment 149177 [details]
Updated Patch
Created attachment 149178 [details]
mylyn/context/zip
Created attachment 149197 [details]
update
If activity exists, preference enabled initially, otherwise disabled. Enablement ui needs refinement.
Created attachment 149198 [details]
mylyn/context/zip
Created attachment 149287 [details]
in progress
Created attachment 149345 [details]
patch
Shawn, can you take a look at the changes to these classes? * ContextCorePlugin * ContextUiPlugin * MonitorUiPlugin * ActivityExternalizationParticipant * TaskActivationExternalizationParticipant * TaskActivityMonitory * TasksUiPlugin The UI changes should be fairly straight forward. The changes look like they should be OK. The preferences in monitor and context seem a bit odd, but it looks like it is probably the cleanest way to do it. Note that the patch didn't apply cleanly due to the changes to the private section. I have applied the patch locally and will let you know if I see any oddities. Created attachment 149399 [details]
updated patch
I have fixed up a race condition that could cause time tracking not to get activated. Based on Shawn's suggestion I have also moved handling of activity tracking enablement from ContextCorePlugin to MonitorUiPlugin. Please note that the patch causes the activity job to always get started. Previously that required at least a single task activation. Rob, can you explain the reason for this change? I have committed the patch please review my changes carefully. Rob, please take a look at this test failure: junit.framework.ComparisonFailure: expected:<[star]ted> but was:<[activa]ted> at junit.framework.Assert.assertEquals(Assert.java:81) at junit.framework.Assert.assertEquals(Assert.java:87) at org.eclipse.mylyn.java.tests.InteractionContextManagerTest.testShellLifecycleActivityStart(InteractionContextManagerTest.java:110) Created attachment 149482 [details]
show scheduling information in header
Done. (In reply to comment #14) > Please note that the patch causes the activity job to always get started. > Previously that required at least a single task activation. Rob, can you > explain the reason for this change? We should consider adding this back. This was a side effect of simplification but comes at the cost of the job allocation for those who don't activate tasks. (In reply to comment #15) > Rob, please take a look at this test failure: > > junit.framework.ComparisonFailure: expected:<[star]ted> but was:<[activa]ted> > at junit.framework.Assert.assertEquals(Assert.java:81) > at junit.framework.Assert.assertEquals(Assert.java:87) > at > org.eclipse.mylyn.java.tests.InteractionContextManagerTest.testShellLifecycleActivityStart(InteractionContextManagerTest.java:110) I'll take a look (In reply to comment #18) > (In reply to comment #14) > > Please note that the patch causes the activity job to always get started. > > Previously that required at least a single task activation. Rob, can you > > explain the reason for this change? > We should consider adding this back. This was a side effect of simplification > but comes at the cost of the job allocation for those who don't activate tasks. The job should run if there is an interested consumer, e.g. time tracking or task list sync. It should not be based on a preference. (In reply to comment #19) > (In reply to comment #18) > > (In reply to comment #14) > > > Please note that the patch causes the activity job to always get started. > > > Previously that required at least a single task activation. Rob, can you > > > explain the reason for this change? > > We should consider adding this back. This was a side effect of simplification > > but comes at the cost of the job allocation for those who don't activate > tasks. > > The job should run if there is an interested consumer, e.g. time tracking or > task list sync. It should not be based on a preference. Agreed. I'll look at adding this now. As for the failing unit test, when run with TEST_MODE = true, the necessary bits run in the the ui job correctly which results in correct startup and sequence of activity events. Thoughts on how to test this? Can we run a single test as 'non-test' mode? (In reply to comment #20) > (In reply to comment #19) > > (In reply to comment #18) > > > (In reply to comment #14) > > > > Please note that the patch causes the activity job to always get started. > > > > Previously that required at least a single task activation. Rob, can you > > > > explain the reason for this change? > > > We should consider adding this back. This was a side effect of > simplification > > > but comes at the cost of the job allocation for those who don't activate > > tasks. > > > > The job should run if there is an interested consumer, e.g. time tracking or > > task list sync. It should not be based on a preference. > Agreed. I'll look at adding this now. We now rely on these events for context externalization and optimization of synchronization so will leave job as is for now. > > The job should run if there is an interested consumer, e.g. time tracking or > > task list sync. It should not be based on a preference. > Agreed. I'll look at adding this now. > > As for the failing unit test, when run with TEST_MODE = true, the necessary > bits run in the the ui job correctly which results in correct startup and > sequence of activity events. Thoughts on how to test this? Can we run a single > test as 'non-test' mode? We should review that code, maybe the special handling in start() can be removed. I have created bug 292206 to track that. |