[VCSIface] Difference between VcsEvent and VcsItemEvent

[Matthew Woehlke]

Andreas Pakulat wrote:
> On 07.05.07 09:54:01, Matthew Woehlke wrote:
>> Andreas Pakulat wrote:
>>> I don't quite understand the purpose of these two classes and especially
>>> why they are separated. Would someone please enlighten me? (and write
>>> more verbose apidox)
>> Done (the apidox).
> 
> Thanks.
> 
>> "[VcsItemEvent] contains information that is common to a VcsEvent (which 
>> represents a single item) and a VcsChange (which may have information 
>> about several items)."
>>
>> Does that help? Basically you get a VcsEvent from log() and a VcsChange 
>> from change().
> 
> Why?

Because they return different information? (And 'why do they do that' 
was your next question, so...)

>> log() doesn't tell you anything about other files 
>> affected at the same time, for that you use change().
> 
> Why?

Because I don't expect callers of log() to often care about this, and to 
get it from perforce is orders-of-magnitude more difficult. First you 
use 'p4 filelog' to find out about the file. Then you use 'p4 describe' 
to find out about each change that 'p4 filelog' tells you about. Then 
you use 'p4 filelog' again on every item/revision pair to find out 
correctly what the action is. So instead of one action, log() now has to 
issue three in sequence.

>> VcsItemEvent holds 
>> the per-item properties in VcsChange (i.e. the revision number, date, 
>> user and message are the same for all items affected by a change :-)).
>>
>> Hmm. Certain VCS's (perforce) have a log() that doesn't tell you about 
>> peripheral changes. Is it preferable to have two classes, or would you 
>> prefer having only VcsChange with the stipulation that log() may give 
>> you an incomplete instance (i.e. only the item you asked about?).
> 
> IMHO we should look at having 2 classes in sum, one for the change and
> one for the separate items in the change. Then we also don't need
> change() anymore as for getting 1 revision all you need to do is
> log(path,revision,1).

Except that perforce doesn't work that way. If you want to know about a 
change, you have to ask about a change. If you want to know about a 
file, you have to ask about a file. You can't find out about change 
12345 asking about foo.c if foo.c was not affected by change 12345. (Ok, 
so assuming you know what the repository root is, it's good* guess the 
repository root changed at any given change.) So I would prefer to keep 
change().

(* but NOT entirely safe, as you might expect, since change numbers can 
vanish. This usually happens because named change sets eat out of the 
global number, so when you commit them, if anything has happened since 
their creation, the number assigned to a change set at creation ceases 
to be associated with anything.)

I'd be OK making log() have the same result type, but that leaves four 
options:

1. Have inconsistent behavior as to whether or not you get "peripheral 
information" from log().
1a. This means that if you want to track a single file, you have to do 
it yourself. Bummer.
2. Always return information about /only/ the requested file, even if 
more information is available
2a. This moves the burden of 1a from user-space to the plugin.
3. Make perforce's log() really, really slow due to having to execute an 
additional operation for every result returned by log().
3a. ...and we still need change() unless you want to log() the 
repository root.
4. Combine 1 and 3 (eliminating 1a) by adding a flag specifying if you 
want to know about the changes as well.

>> Or is it preferable to keep the separation because it shifts the load
>> of sorting out the path of a single file to the plugin (in the case of
>> log(), on VCS's - i.e. svn - that make this necessary) rather than the
>> user?
> 
> I don't have the classes at hand atm, so I won't comment on this atm -
> not understanding what you say...
> 
>> I think I chose not to make VcsEvent a subclass of VcsItemEvent to make 
>> it easier for plugins to subclass in order to provide additional 
>> information.
> 
> Thats possible even if it is a subclass. Or rather these should be
> interfaces if we really keep both.

Making it an interface would work, although I assume the implementation 
is trivial in this instance, it's just accessors for member data, i.e. a 
glorified struct.

-- 
Matthew
child: Do not try to read the .sig. That is impossible. Instead you must 
realize the truth.
init: What truth?
child: There is no .sig.