Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hlint suggestions #260

Merged
merged 16 commits into from
Dec 18, 2017
Merged

Hlint suggestions #260

merged 16 commits into from
Dec 18, 2017

Conversation

kazu-yamamoto
Copy link
Collaborator

This PR accepts reasonable hlint suggestions.

@kazu-yamamoto
Copy link
Collaborator Author

I have rebased this PR onto master.

Copy link
Contributor

@ocheron ocheron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

90 suggestions down, not bad!

@@ -314,7 +311,7 @@ onServerHello ctx cparams sentExts (ServerHello rver serverRan serverSession cip

-- intersect sent extensions in client and the received extensions from server.
-- if server returns extensions that we didn't request, fail.
when (not $ null $ filter (not . flip elem sentExts . (\(ExtensionRaw i _) -> i)) exts) $
unless (null $ filter (not . flip elem sentExts . (\(ExtensionRaw i _) -> i)) exts) $
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't this be further simplified with all?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we can. But I don't know whether or not @vincenthz likes it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a fan of when not -> unless, it's much less readable IMHO, but if you guys are happy with those changes, let's move forward. same goes for all. I either will adapt myself to the syntax or go mad :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep this as is.

kazu-yamamoto added a commit to kazu-yamamoto/hs-tls that referenced this pull request Dec 18, 2017
@kazu-yamamoto kazu-yamamoto merged commit 40f4a80 into haskell-tls:master Dec 18, 2017
@kazu-yamamoto kazu-yamamoto deleted the hlint branch December 18, 2017 13:25
@kazu-yamamoto
Copy link
Collaborator Author

Merged. Thank you for reviewing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants