Community
Participate
Working Groups
While time tracking is optional in the sense that you currently don't need to use it, and the information is private, but you cannot use both focusing and time tracking since we don't have a separate preference. Proposal: * On the "Tasks" preference page add a check box that says "Track time spent on active task". * Make this new preference off by default so that we give developers a better sense of control by opting into this functionality. * If the option is off, do not show the "Active" text on the Private task editor section. * If the option is off, do not write the heartbeat events that we use to determine time tracking information.
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.
