Hi,
1/ true and agree. I write it pretty quickly. Could you (or someone else)
write that configuration stuff?
2/ yes. thanks to point it out!
3/ I want to simulate same behavior as Visual Studio. Dependancy on registry
read could be problem but if you use <solution> task it is very propable
that you use machine with Visual Studio. Of course - it _should_ be ok to
run it on mono as well (skiping registry read?). Your idea with
includevsfolders atribute is good, but I still think that default value
should be true.
Thanks you, Jarek, for review. I'll revise my code today and address all
your comments.
Martin
----- Original Message -----
From: "Jaroslaw Kowalski" <[EMAIL PROTECTED]>
To: "Martin Aliger" <[EMAIL PROTECTED]>; "! nant"
<[EMAIL PROTECTED]>
Sent: Monday, September 01, 2003 5:48 PM
Subject: Re: [nant-dev] solution task & assemblyfolders
> Hi all!
>
> That's a very good idea, however just by looking at your patch I see few
> problems:
>
> 1. When you have both VS.NET 2002 and VS.NET 2003 you actually have more
> registry keys to consider:
>
> HKEY_LOCAL_MACHINE\Software\Microsoft\VisualStudio\7.0\AssemblyFolders
> HKEY_CURRENT_USER\Software\Microsoft\VisualStudio\7.0\AssemblyFolders
> HKEY_LOCAL_MACHINE\Software\Microsoft\VisualStudio\7.1\AssemblyFolders
> HKEY_CURRENT_USER\Software\Microsoft\VisualStudio\7.1\AssemblyFolders
>
> where as a rule CURRENT_USER settings override the local machine. I think
> that the registry paths should be stored a part of the framework
> configuration in NAnt.exe.config. For mono on linux you would simply have
no
> paths to examine.
>
> 2. Looks like there's a mistake in your code:
>
> + FileSet af = _solutionTask.AssemblyFolders;
> + if(af.DirectoryNames.Count==0)
> af=_solutionTask.DefaultAssemlyFolders;
> +
> + foreach(string path in
> _solutionTask.AssemblyFolders.DirectoryNames)
> + {
> + fiRef = new FileInfo(Path.Combine(path,justFilename));
> + if ( fiRef.Exists )
> + {
> + _referenceFile = fiRef.FullName;
> + _baseDirectory = fiRef.DirectoryName;
> + return;
> + }
> + }
>
> You initialize local variable "af" to some fileset but iterate over
> "_solutionTask.AssemblyFolders.DirectoryNames" so obviously "af" is not
> used. I think it was meant to be "foreach(string path in
af.DirectoryNames)"
>
> 3. I think there should be an option to include/exclude
> "DefaultAssemblyFolders" from processing. In your patch there's a line:
>
> + if(af.DirectoryNames.Count==0)
> af=_solutionTask.DefaultAssemlyFolders;
>
> I think that the semantics should be: "process all directories specified
in
> the fileset PLUS any directories found via DefaultAssemblyFolders when the
> user asks for it"
>
> You would have a syntax like this.
>
> <solution includevsfolders="true">
> </solution>
>
> I think that this new option should default to "false". Having it
otherwise
> would be dangerous as it would by default introduce a dependency of the
> registry and might produce inconsistent results on different build
machines.
>
> Overall: the direction is good, but your patch still needs some work,
> Martin. You're very close, though.
>
> Jarek
>
> ----- Original Message -----
> From: "Martin Aliger" <[EMAIL PROTECTED]>
> To: "! nant" <[EMAIL PROTECTED]>
> Sent: Monday, September 01, 2003 5:04 PM
> Subject: [nant-dev] solution task & assemblyfolders
>
>
> > Hi all!
> >
> > I implement that <assemblyfolder> subtag for <solution> as we discused
it
> > here. I hope my coding convention is ok (try to be close to yours). I'm
> also
> > totally confused by security checks. Have you any experience with
enabling
> > read from registry? [it is ok for me without it, but...]
> >
> > There will be some other matters regarding project dependencies and
paths
> > but I need to explore it first.
> >
> > Regards,
> > Martin
> >
>
>
>
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
nant-developers mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/nant-developers
