Bug 335421

Comment 1 Steffen Pingel 2011-01-29 18:17:28 EST

+1 That sounds like a very useful addition to enable linking between tasks and revisions.

Comment 2 Alexander Gurov 2011-01-30 11:17:00 EST

Hi!

I have checked plug-in from the issue #306790 and it seems there are some problems regarding SVN integration. If I correctly understood, the following code fragment is supposed to install monitor for all the actions performed over the selected repository location. Is this correct?

ISVNConnector proxy = location.acquireSVNProxy();
try
{
   SVNUtility.addSVNNotifyListener(proxy, this);
}
finally
{
   location.releaseSVNProxy(proxy);
}

If so, there goes the actual problem: 

Subversive handles few (no more than 5) cached instances of the SVN connectors for each repository location (in order to spare connector initialization time). If there are more simultaneous jobs than 5, then new instances will be produced and then, after the jobs are finished, all the instances exceeding limit of 5 cached will be destroyed when location.releaseSVNProxy(proxy) method is called. It means that your code installs listener to 1 or to none out of all the 5 cached connector instances (depending on the situation in runtime). So, you will inevitably lose some events.

And then it finally leads us back to the issue #327399.

Regarding History View configuring issue:

	public void setStartRevision(SVNRevision startRevision)
	public SVNRevision getStartRevision()
	public void setEndRevision(SVNRevision endRevision)
	public SVNRevision getEndRevision()

I hope this function set will be convenient?

Comment 3 Alexander Gurov 2011-01-30 11:18:41 EST

If there is anything else regarding History View, please let me know.

Comment 4 Eike Stepper 2011-01-31 13:37:16 EST

(In reply to comment #2)
> Hi!
> 
> I have checked plug-in from the issue #306790 and it seems there are some
> problems regarding SVN integration. If I correctly understood, the following
> code fragment is supposed to install monitor for all the actions performed over
> the selected repository location. Is this correct?

Yes, that was my intent, in the lack higher level hooks.

> ISVNConnector proxy = location.acquireSVNProxy();
> try
> {
>    SVNUtility.addSVNNotifyListener(proxy, this);
> }
> finally
> {
>    location.releaseSVNProxy(proxy);
> }
> 
> If so, there goes the actual problem: 
> 
> Subversive handles few (no more than 5) cached instances of the SVN connectors
> for each repository location (in order to spare connector initialization time).
> If there are more simultaneous jobs than 5, then new instances will be produced
> and then, after the jobs are finished, all the instances exceeding limit of 5
> cached will be destroyed when location.releaseSVNProxy(proxy) method is called.
> It means that your code installs listener to 1 or to none out of all the 5
> cached connector instances (depending on the situation in runtime). So, you
> will inevitably lose some events.

That was my suspicion. Does that mean there's currently *no* reliable way to listen for these events, as the connector cache is an invisible implementation detail?

> And then it finally leads us back to the issue #327399.

Does that mean you're willing to address bug 327399 eventually?

> Regarding History View configuring issue:
> 
>     public void setStartRevision(SVNRevision startRevision)
>     public SVNRevision getStartRevision()
>     public void setEndRevision(SVNRevision endRevision)
>     public SVNRevision getEndRevision()
> 
> I hope this function set will be convenient?

I would prefer a single setter with both range end arguments. I assume that a setter call with effective changes of the existing range will implicitely call refresh(xxx), as well. Then it would be nicer for the UI if both ends of the range could be set before a single refresh is triggered.

Can you please explain if/how you plan to support settng the range *before* the view/page is shown the first time. It seems as if at the first time an asynchronous refresh is automatically triggered with the default range, e.g. HEAD til HEAD-25. It would be need to bypass this (for us) useless refresh. Please also note that a (reflective) call to refresh during an ongoing refresh seems to be ignored and the end of the ongoing refresh is unpredictable.