On Mon, Aug 03, 2015 at 01:48:45PM -0600, Peter Harpending wrote: > Patch review time! > > On Sun, Aug 02, 2015 at 11:26:14PM +0300, [email protected] wrote: > > > + let libravatar email = avatarUrl (Left email) False Nothing Nothing > > + avatarFinal <- liftIO $ > > + case (userAvatar user, userEmail user, userEmail_verified user) of > > + (Just url, _, _) -> return $ Just url > > + (Nothing, Just email, True) -> do > > + murl <- libravatar $ T.unpack email > > + return $ murl >>= return . fromString > > I am entirely sure this last line can be written more clearly. How about > > return (Just (fromString murl))
Close, but no cigar. :) Type mismatch and other infelicities. I
suggest using the Functor instance:
return (fmap fromString murl)
> > + _ -> return Nothing
> Same thing here with aligning delimiters against variable-width
> blocks of text.
I almost agree. I'm on the fence about aligning delimiters against
variable-width blocks of text, but I definitely think nothing is
gained in doing so if there are intermediate lines that begin
far to the left of the aligned delimiter. I should stress this is
purely an issue of style, but hey, somebody has to make the rules....
Besides these purely-stylistic things I think the patch looks good.
I'll merge it. Thanks!
signature.asc
Description: Digital signature
_______________________________________________ Dev mailing list [email protected] https://lists.snowdrift.coop/mailman/listinfo/dev
