[Mono-dev] System.dll few patches for review

[Sebastien Pouliot]

Hello Andrew,

On Tue, 2006-10-03 at 10:28 -0700, Andrew Skiba wrote:
> Hello Sebastien,
> 
> > > Part of them is needed to omit TARGET_JVM, so code will be common.
> > 
> > There are no TARGET_JVM in the two files.
> > 
> 
> As I said, these changes are needed to omit TARGET_JVM.

oops

> > > * DigestClient.patch - use MD5.Create instead of 
> > HashAlgorithm.Create
> > > ("MD5")
> > 
> > This is ok in the sense that it will result in the exact same 
> > thing. So the question is: Is that change required ? if so, 
> > can you say why (and include the rational in the ChangeLog).
> 
> After futher investigation, I see that this old limitation in
> Grasshopper is gone, so this patch is not necessary anymore.

k

> > > * X509CertificateCollection.patch - remove unnecessary overload
> > 
> > If this doesn't cause any error with the class library status 
> > pages then remove (don't comment) it. The comment itself can 
> > be put in the ChangeLog.
> 
> According to MSDN X509CertificateCollection does not implement
> IEnumerable privately. 

You're right. Anyway CollectionBase already implements IEnumerable. Not
sure why it was put there (it's been there since the first commit in
2002).

> Actually, that means that the patch should look
> like in the new attachment.
> 
> What do you mean by class library status pages? 

The "old corcompare" which is available online from
http://www.mono-project.com/Class_Status

You should check both 1.x and 2.0 profiles.

> I could run make
> run-test after this patch applied, and it gave same number of errors
> before and after the patch. Is that enough?

No. It will spot functionality regressions but it won't spot errors in
API definitions. I don't see how/why this could break but it's safer to
always check the pages after an API change (either manually on your own
computer or on the public pages a while after the check-in).

-- 
Sebastien Pouliot  <sebastien at ximian.com>
Blog: http://pages.infinit.net/ctech/