[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-dev] PRELIMINARY: [PATCH 3/3] Replace 'TorDNSEL.System.Timeout' with 'System.Timeout'.



> Sorry for being slow to get to your patches.

No problem.  I'm glad that someone is actually interested in that.

> An overall comment: I am unconvinced about commit messages that details
> obvious changes for each impacted file.

Well, I agree.  However, even obvious changes might be useful in the
long term.  The GNU Coding Standards, which I use as a guide, suggest
the following: "Subsequent maintainers will often search for a function
name to find all the change log entries that pertain to it..." [1]  (For
instance, cgit allows that.)  It also helps when you're trying to fix
bugs.  Moreover, I often spot mistakes when I'm writing such messages.

Sometimes it's harder to write commit messages than code.  So if you
think that they are verbose, you're probably right.

>> 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.

Unused, right.

> 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".

Attached.  Do you have any other comments?

[1] https://www.gnu.org/prep/standards/standards.html#Style-of-Change-Logs

From 87ad0fe6f010b251aed042618dc82651294a477a Mon Sep 17 00:00:00 2001
From: Nikita Karetnikov <nikita@xxxxxxxxxxxxxx>
Date: Thu, 25 Jul 2013 20:29:08 +0000
Subject: [PATCH 1/2] Reimport internal GHC functions from new locations.

---
 src/TorDNSEL/Util.hsc |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/TorDNSEL/Util.hsc b/src/TorDNSEL/Util.hsc
index 0ba7365..8cfe172 100644
--- a/src/TorDNSEL/Util.hsc
+++ b/src/TorDNSEL/Util.hsc
@@ -139,7 +139,8 @@ import System.Posix.Types (FileMode)
 import Text.Printf (printf)
 
 import GHC.IO.Handle (BufferMode(..))
-import GHC.IOBase (Handle, Handle__(..), readIORef, writeIORef)
+import GHC.IO.Handle.Types (Handle, Handle__(..))
+import GHC.IORef (readIORef, writeIORef)
 
 import Data.Binary (Binary(..))
 
-- 
1.7.9.5

From c9907a1f4e939417d1a77adcb27501c6506efe8a Mon Sep 17 00:00:00 2001
From: Nikita Karetnikov <nikita@xxxxxxxxxxxxxx>
Date: Thu, 25 Jul 2013 20:33:13 +0000
Subject: [PATCH 2/2] Remove the unused 'splitByDelimiter' function.

---
 src/TorDNSEL/Util.hsc |   13 -------------
 1 file changed, 13 deletions(-)

diff --git a/src/TorDNSEL/Util.hsc b/src/TorDNSEL/Util.hsc
index 8cfe172..f71cf99 100644
--- a/src/TorDNSEL/Util.hsc
+++ b/src/TorDNSEL/Util.hsc
@@ -3,8 +3,6 @@
              UndecidableInstances, FlexibleInstances, MultiParamTypeClasses,
              GeneralizedNewtypeDeriving, FlexibleContexts #-}
 {-# OPTIONS_GHC -fno-warn-type-defaults -fno-warn-orphans -Wwarn #-}
---                                                        ^^^^^^
---                                    XXX: findSubstrings is deprecated
 
 -----------------------------------------------------------------------------
 -- |
@@ -59,7 +57,6 @@ module TorDNSEL.Util (
   , htonl
   , ntohl
   , hGetLineN
-  , splitByDelimiter
   , showException
   , showUTCTime
 
@@ -375,16 +372,6 @@ hGetLineN handle eol n = do
   bStr <- B.hGet handle n
   return $ fst $ B.breakSubstring eol bStr
 
--- | Split @bs@ into pieces delimited by @delimiter@, consuming the delimiter.
--- The result for overlapping delimiters is undefined.
-splitByDelimiter :: ByteString -> ByteString -> [ByteString]
-splitByDelimiter delimiter bs = subst (-len : B.findSubstrings delimiter bs)
-  where
-    subst (x:xs@(y:_)) = B.take (y-x-len) (B.drop (x+len) bs) : subst xs
-    subst [x]          = [B.drop (x+len) bs]
-    subst []           = error "splitByDelimiter: empty list"
-    len = B.length delimiter
-
 -- | Convert an exception to a string given a list of functions for displaying
 -- dynamically typed exceptions.
 showException :: [Dynamic -> Maybe String] -> E.SomeException -> String
-- 
1.7.9.5

Attachment: pgpXxJN8e8sp9.pgp
Description: PGP signature

_______________________________________________
tor-dev mailing list
tor-dev@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-dev