Thanks henrik. I'm in the process of integrating the changes :) I was just systematically converting. So I will create a new issue for these remarks.
Stef On Feb 4, 2010, at 10:02 PM, Henrik Sperre Johansen wrote: > On 03.02.2010 10:26, Stéphane Ducasse wrote: >> Hi henrik >> >> if you get a chance to have a look at the slice I uploaded it would be good. >> I like the idea that people have a look at other code, which is what we do >> when we integrate >> now for my own code I always feel nervous :) >> >> Stef >> > Got time tonight, here's my comments: > > DigitalSignatureAlgorithm class >> testExamplesFromDisk > May be better as a real test than as a class-side method? The method > usage change seems fine though ;) > > FilePackage >> fromFileNamed: > Should be inlined in the class-side method called the same, and removed. > Only this method is ever called anyways, and it makes less sense to > accept from... style initialization methods on the instances. > > > MCVersionReader class >> file:streamDo: > Could instead be inlined in versionFromFile which is the only sender, > then removed. > The way it's now, it's really only a MCVersionReader-specific alias for > the exact same method :) > > MCVersionReader class >> versionFromFile: > ^FileStream readOnlyFileNamed: fileName do: [:fileStream | self > versionFromStream: fileStream] > > > UUIDGenerator>>makeUnixSeed > You set the stream to binary, there should be no need to set the > converter as well (it won't be used in binary mode). > > Other than that, the changes look ok to me, and make the methods easier > to read (imo) > > Cheers, > Henry > > _______________________________________________ > Pharo-project mailing list > Pharo-project@lists.gforge.inria.fr > http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project _______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
