Re: [Anjuta-devel] Gnome-VCS API discussion

[Jeroen Zwartepoorte <jeroen xs4all nl>]

Hi Naba,

On Tue, 2003-09-09 at 11:53, Naba Kumar wrote:
> On Tue, 2003-09-09 at 14:12, Jeroen Zwartepoorte wrote:
> > Hi Johannes,
> > 
> > On Mon, 2003-09-08 at 15:21, Johannes Schmid wrote:
> > > Hi!
> > > 
> > > I have put my first proposal of a Gnome-VCS API on http://www.anjuta.
> > > org/jhs/gnome-vcs . Please comment on it. Anything is welcome. Of 
> > > course the code will go to a CVS repositry sometime later.
> > 
> > Here are some comments after looking at the source files:
> > 
> > - Are the callbacks you have defined as part of the GnomeVCSClass
> > _really_ callbacks or are they signals? I'm asking because callbacks
> > shouldn't be part of the Class.
> 
> Just curious. Did you mean callbacks == virtual functions?

Yeah, they are virtual functions in the current source, but they should
be callbacks imo.

> > - Do the file methods really need a message parameter? For CVS these
> > operations don't affect the repository. You only need to provide a
> > message once you commit the changes.
> > 
> CVS has the option to provide the message, but I don't know what is it
> really useful for.
> 
> > - Does file_update really need a branch parameter? Don't have you to
> > specify a branch when you checkout a module?
> > 
> Yes, it does (although, rarely). When we want to switch branches in the
> checked out version of the source, we do update with the given branch.
> This makes sure our current modifications (if any) stay in the updated
> version and to avoid doing fresh checkouts. Passing NULL, should of
> course update the current branch.

Right.

> > - Instead of having const gchar *message parameters in almost all
> > methods (do VCS system other than CVS really require them?), wouldn't it
> > be better to have the recipient register a callback that tells it when
> > the VCS backend requires a message? The recipient can then popup a
> > dialog asking for a message from the user.
> > 
> I think the better approach is to have it passed to all the methods
> requiring it (optioanlly or otherwise). If a NULL is passed during the
> invocation of the methods, we can then emit a signal to retrieve the
> message. Otherwise message passed is used.

That's just overly complicated imho. Either we always pass a message (or
NULL if none is specified) _or_ we emit a signal/use a callback. Not
both.

> This is because, we do not want to assume the messages will be always a
> user feedback (although that's how it is in normal circumstances). The
> commit should be donable by automated programs handling the CVS
> operations. In such cases, if we rely on a signal for providing the
> message, it will put unnecessary burden of maintaining the state of the
> operation at the automated-program's end to make sure the currect
> message is passed.

My concern is just that not all VCS systems might use/require messages.
So having a message parameter in the abstract class would be a bad idea
(it would basically force the frontend to always ask/provide a message
when calling the methods). Making the backend tell the recipient that it
needs a message seems a better idea.

> The signal emission can also prepare and pass the list of modified files
> (just like command line CVS).

Right. (aside from the issue whether you use a signal or callback). For
some things, callbacks are more useful. Signals need to do marshalling
etc. But for things like message_needed, a signal seems like a good
choice.

> Also let us not complicate things with 'Projects' and 'Files' in
> GnomeVCS. There is no projects in CVS (or other VCS implementations, I
> suppose) and therefore there is no reason we should have it in GnomeVCS.
> There are only files and directories. We don't even need to distinguish
> between files and dirs also. Just have, for example, _commit() instead
> of _commit_project() and _commit_files().

Agreed. We just need a parameter that's basically a list of files: if
you pass NULL, then you want to update/commit etc. the entire module.
Otherwise only perform the operation on the files listed. (don't start
methods in an abstract class with an underscore though: underscore is
generally used to indicate that a method is private).

Jeroen