[Mono-dev] [PATCH] System.Security: trivial fixes for XmlDsig transforms

[Sebastien Pouliot]

On Thu, 2008-08-14 at 04:52 +0200, Gert Driesen wrote:
> Hey Sebastien,
> 
> I'm remove all unnecessary changes (like the brace position, bad
> habits ...) and resubmit the patch for review.

Thanks!
Sebastien


> Gert
> 
> -----Original Message-----
> From: mono-devel-list-bounces at lists.ximian.com [mailto:mono-devel-list-bounces at lists.ximian.com] On Behalf Of Sebastien Pouliot
> Sent: donderdag 14 augustus 2008 2:58
> To: Gert Driesen
> Cc: mono-devel-list at ximian.com
> Subject: Re: [Mono-dev] [PATCH] System.Security: trivial fixes for XmlDsig transforms
> 
> On Sat, 2008-08-09 at 11:04 +0200, Gert Driesen wrote:
> > Hi,
> > 
> > The attached patch provides some trivial fixes for XmlDsig transforms in
> > System.Security, and adds/improves unit tests.
> > 
> > Let me know if this is ok to commit.
> 
> No. It seems there's good stuff (like updating the old syntax of unit
> tests) but then there's all the other unneeded stuff part of this patch
> (some examples follows).
> 
> > 
> > 
> > 
> > 
> > 
> > differences between files attachment (xmldsig.patch)
> > 
> > Index: Test/System.Security.Cryptography.Xml/XmlDsigXsltTransformTest.cs
> > ===================================================================
> > --- Test/System.Security.Cryptography.Xml/XmlDsigXsltTransformTest.cs   (revision 110022)
> > +++ Test/System.Security.Cryptography.Xml/XmlDsigXsltTransformTest.cs   (working copy)
> > @@ -23,8 +23,17 @@
> >  
> >         // Note: GetInnerXml is protected in XmlDsigXsltTransform making it
> >         // difficult to test properly. This class "open it up" :-)
> > -       public class UnprotectedXmlDsigXsltTransform : XmlDsigXsltTransform {
> > +       public class UnprotectedXmlDsigXsltTransform : XmlDsigXsltTransform
> > +       {
> 
> Your interpretation was different than mine so you asked (I think I
> suggested it) on mono-dev about this (May 26th) and Miguel's answer (May
> 27th) was clear: 
>         "Same line, there are a couple of samples on that page."
> 
> Anyway please do not introduce unneeded changes into your patches.
> 
> > +               public UnprotectedXmlDsigXsltTransform ()
> > +               {
> > +               }
> >  
> > +               public UnprotectedXmlDsigXsltTransform (bool includeComments)
> > +                       : base (includeComments)
> > +               {
> > +               }
> > +
> >                 public XmlNodeList UnprotectedGetInnerXml () {
> >                         return base.GetInnerXml ();
> >                 }
> > @@ -36,14 +45,28 @@
> >                 protected UnprotectedXmlDsigXsltTransform transform;
> >  
> >                 [SetUp]
> > -               protected void SetUp () 
> > +               protected void SetUp ()
> 
> This is another case that should not be in the patch (unneeded change).
> Yes I too dislike the extra space after () but let's not disturb those
> bits on everyone boxes.
> 
> ...
> 
> >  
> > -               private XmlDocument GetDoc () 
> > +               XmlDocument GetDoc ()
> 
> Another example of an unneeded change. private is not required but it's
> there (so let it be).
> 
> ...
> 
> > @@ -168,7 +260,7 @@
> >  
> >                 [Test]
> >  #if NET_2_0
> > -               [Category ("NotDotNet")]
> > +               [Category ("NotDotNet")]
> 
> I can't see why this change is needed, so I assume it's about line
> endings ?!?
> 
> If this is required please let's do the line endings changes in a
> separate commit (after ensuring it does not affect unit tests) and
> generate the patch afterward.
> 
> ...
> 
> 
> Sebastien
> 
> _______________________________________________
> Mono-devel-list mailing list
> Mono-devel-list at lists.ximian.com
> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>