[tor-dev] PRELIMINARY: [PATCH 3/3] Replace 'TorDNSEL.System.Timeout' with 'System.Timeout'.
Lunar
lunar at torproject.org
Thu Jul 25 15:37:51 UTC 2013
Hi Nikita,
Sorry for being slow to get to your patches. They are much appreciated
and even needed now that checks.tpo has seen repetitive failures.
An overall comment: I am unconvinced about commit messages that details
obvious changes for each impacted file.
For example:
> * src/TorDNSEL/Util.hsc: Use 'GHC.IO.Handle.Types' and 'GHC.IORef'
> instead of 'GHC.IOBase'.
> (splitByDelimiter): Remove it.
Both are completly obvious by looking at the commit diff, so there is no
added value in such message. This single commit should probably be split
in two, one saying "update imports to the new locations of internal
GHC functions" and another one saying "remove the unused
splitByDelimiter function".
I am not sure I completly understand all the intents, so I might have
misunderstood the intents.
Nikita Karetnikov:
> The first one deals with this issue: "GHC was recently changed to not
> allow you to use newtypes in FFI imports unless the constructor of the
> newtype is in scope."
This one looks good.
> The second patch removes a redundant function and adjusts some imports.
> (I wrote a version of 'splitByDelimiter' that uses 'B.breakSubstring'
> instead of 'B.findSubstrings'. Let me know if you want to keep
> 'splitByDelimiter', and I'll use that version.)
Redundant or unused? I don't see any changes other than removing the
function.
> The third patch removes 'TorDNSEL.System.Timeout'. There is a similar
> module in 'base'. […] There is another reason to remove
> 'TorDNSEL.System.Timeout': it's the only module that is not in the
> public domain.
I think this is because this module is actually based on an earlier
version that what finally landed in base. Switching to that makes a lot
of sense.
--
Lunar <lunar at torproject.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.torproject.org/pipermail/tor-dev/attachments/20130725/412c057d/attachment.sig>
More information about the tor-dev
mailing list