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

Make c2hs a separate toolchain. #590

Merged
merged 1 commit into from
Jan 18, 2019
Merged

Make c2hs a separate toolchain. #590

merged 1 commit into from
Jan 18, 2019

Conversation

mboes
Copy link
Member

@mboes mboes commented Jan 17, 2019

The GHC toolchain could previously optionally be provided an extra
"tool", c2hs. But

This is a breaking change. But few people use c2hs and the change
is only in the toolchain declaration code, which happens only once per
workspace.

Copy link
Contributor

@Profpatsch Profpatsch left a comment

Choose a reason for hiding this comment

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

Looks good! Was this necessary because c2hs doesn’t work on Windows?

Update: Looks like it. https://github.com/tweag/rules_haskell/pull/581/files#diff-a376c47dc40424f31d9cd7b3543e4850R23

register_toolchains("//:c2hs")
```

"""
Copy link
Contributor

Choose a reason for hiding this comment

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

A new line between the doc and the implementation?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the default Emacs style for docstrings in Python. Can't say I'm a fan. But we already have this in a few places in the code.

The GHC toolchain could previously optionally be provided an extra
"tool", c2hs. But

This is a **breaking change**. But few people use c2hs and the change
is only in the toolchain declaration code, which happens only once per
workspace.
@mboes mboes merged commit 86f3eee into master Jan 18, 2019
@mboes mboes deleted the c2hs-toolchain branch January 18, 2019 19:35
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