[MonoDevelop] Some more code for version control support

Rubens Ramos rubensr@users.sourceforge.net
Tue, 07 Dec 2004 17:46:52 +1030


Hi Lluis,

 First, thanks a lot for your comments. Just a few things:

>       * VcService is using a copy of the GetXmlAttributes and
>         SetXmlAttributes from AbstractProject. We need to factorize this
>         code into a class so it can be easily reused (although that code
>         is really awful). 
I absolutely agree, the reason I copied the code is I wasn't quite
sure where to put this new class, or what would be its main purpose.
Any suggestions?

>       * Project specific information should always be stored in the
>         project's directory, never in a global shared file. 
The only project information stored in the VC file concerns the
repository, which can be shared among various projects - this
is inspired by the way things are handled in Eclipse. Another
reason to keep this stuff out of the project file was to
keep passwords/usernames in a separate place.

>       * Looks like the class VcResource is not used at all. 
You are right :)
We will use that once the connection between the backends and the vc
core is working. I think the actual interface that will use it
is in the wiki (bitflux has all that done :)

>       * Why the IVcRepository interface is defining ToString()? This
>         method is already defined in Object. If you want to give a
>         special meaning to that method (such as, for example, getting
>         the uri of the repository) better use another name. 
That was not a good idea on my part - I will change that.

>       * IVcLocalProject.PrjxFile: better use "ProjectFile", since the
>         prjx extension may change in the future. 
Agree.

>       * Since CvsService is actually a version control backend, I think
>         that CvsBackendService would be more clarifying name. 
Agree.

>       * In IVcBackend, it is difficult to understand what's the
>         difference between CreateRepository and RawCreateRepository. I
>         rather would use CreateRepository to create a IVcRepository
>         instance, and a ShowEditRepositoryDialog method that displays
>         the dialog that can be used to enter/modify repository
>         parameters. 
Good suggestion, will do that.

>       * The CvsRepository.GetOptions method (which implements
>         IVcRepository.GetOptions) returns some HTML that shows
>         repository parameters. You are mixing version control logic code
>         with GUI code here, and that's not good design pattern. A better
>         solution would be to return, for example, a NameValueCollection,
>         and leave to the GUI the choice of the best way of showing this
>         info. 
Using the HTML widget was not a good idea in the first place. Because
different backends will need to show different configuration stuff,
there will need to be a method returning a widget, or a method
that receives the optiondialog so that the backend can put its
configuration parameters there. Does that sound ok?

>       * CvsService.CreateConnection is also mixing logic and GUI. You
>         are assuming that a dialog will always be displayed when
>         connecting a project to a repository, and this may not be true.
>         You should have a method for creating a connection object and
>         another for displaying the connection dialog. 
I can certainly split the two, but I am not sure that this will
be a practical improvement - since the VC code does not know how
to manipulate the IVcRepository object at all - and it would probably
be useless to have a method to simply create a repository without
useful configuration parameters.

>       * After going through the code, I think that a IVcRepository is
>         not a version control repository, but a set of repository
>         connection parameters. IVcLocalProject looks like it contains
>         version control parameters for a project. Maybe you should use
>         names that better describe what the class represents. 
Ok - I was also following the wiki examples, so I accept new
name suggestions :)

>       * The cvsOptionsDialog and cvsRepositoryWidget dialogs look very
>         similar, they could probably be the same.
I will look into this.

Thanks again.
Rubens