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

Add an Imports.hs module #440

Merged
merged 3 commits into from
Oct 9, 2018
Merged

Add an Imports.hs module #440

merged 3 commits into from
Oct 9, 2018

Conversation

neongreen
Copy link
Contributor

Just putting it out there so that nobody tries to do it on their own.

Let's definitely not merge it before LTS pull-requests.

@neongreen neongreen requested a review from fisx August 14, 2018 10:30
import Database.Redis.IO hiding (Milliseconds)
import Gundeck.Monad (Gundeck, posixTime)
import Gundeck.Types
import Gundeck.Util.Redis

import qualified Data.ByteString as Strict
Copy link
Contributor

Choose a reason for hiding this comment

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

SBS instead of Strict? not important.

another idea:

$(importQualifiedDefaults)

Which expands to a list of qualified imports we usually want. I guess the problem is that this would trigger unused import warnings? Also not important. :-) And we can always do that another time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TH can't generate imports

Copy link
Contributor Author

@neongreen neongreen Aug 14, 2018

Choose a reason for hiding this comment

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

this problem (preludes can't provide qualified imports) has no solution AFAIK; you can use #includes but then you have to turn off the "unused imports" warning, true

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, i've used CPP in my last project, and the results are inconclusive. I think the other(s) liked it, I was skeptical.

Ok, no default qualified imports for now then!

Copy link
Contributor

@fisx fisx left a comment

Choose a reason for hiding this comment

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

cool!

import Database.Redis.IO

import qualified Data.ByteString.Lazy as BSL
Copy link
Contributor

Choose a reason for hiding this comment

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

(LBS? i like the aliases from string-conversions. :-) )

Copy link
Contributor

@tiago-loureiro tiago-loureiro left a comment

Choose a reason for hiding this comment

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

Note that I don't disapprove but I will request changes to avoid us accidentally merging this in too early.

@neongreen
Copy link
Contributor Author

@tiago-loureiro I think this can be merged now, can you un-reject?

Copy link
Contributor

@tiago-loureiro tiago-loureiro left a comment

Choose a reason for hiding this comment

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

Please re-trigger the build before merging.

@neongreen neongreen merged commit ece56bc into develop Oct 9, 2018
@neongreen neongreen deleted the imports branch October 9, 2018 07:09
king6cong pushed a commit to king6cong/wire-server that referenced this pull request Oct 12, 2018
* Add Imports and migrate cargohold, gundeck, proxy

* Fix warnings

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

None yet

3 participants