Skip to content

Conversation

@HirotoShioi
Copy link
Contributor

@HirotoShioi HirotoShioi commented May 10, 2018

Description

Migrate the code to use universum

Linked Issues

https://iohk.myjetbrains.com/youtrack/issue/CO-280

Branched from this PR

Note

I did follow the migration guide written on the package document but I might have missed something. Please check carefully!

@HirotoShioi HirotoShioi self-assigned this May 10, 2018
@HirotoShioi HirotoShioi force-pushed the co-280-switch-to-universum branch 2 times, most recently from bf37cb5 to 79ab4a5 Compare May 11, 2018 02:46
@HirotoShioi HirotoShioi removed the WIP label May 11, 2018
@HirotoShioi HirotoShioi force-pushed the co-280-switch-to-universum branch from 79ab4a5 to f5006bd Compare May 13, 2018 23:55
@HirotoShioi HirotoShioi force-pushed the co-280-switch-to-universum branch 2 times, most recently from f25c7d2 to 6ca16a7 Compare May 15, 2018 05:29
@HirotoShioi HirotoShioi force-pushed the co-280-switch-to-universum branch from 6ca16a7 to 5c85880 Compare May 15, 2018 08:22
@HirotoShioi HirotoShioi requested a review from ksaric May 17, 2018 07:31
@HirotoShioi HirotoShioi force-pushed the co-280-switch-to-universum branch from 6a1050b to e637a9a Compare May 17, 2018 09:03
Copy link
Contributor

@ksaric ksaric left a comment

Choose a reason for hiding this comment

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

Other than a single comment, LGTM.
We can merge this and I'll fix my PR #9 afterward.

e <- quotedField
_ <- char ','
_ <- char '"'
() <$ char ','
Copy link
Contributor

Choose a reason for hiding this comment

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

Revert this, it's really not that readable. There are two options:

  • use the previous version - _ <- char ',', which I prefer
  • use void - void $ char ','

@HirotoShioi HirotoShioi merged commit 3c30ed5 into master May 17, 2018
@ksaric ksaric deleted the co-280-switch-to-universum branch May 17, 2018 09:52
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