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 CI #13

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add CI #13

wants to merge 2 commits into from

Conversation

BinderDavid
Copy link

Add a Github Action CI Build matrix for Windows + Mac + Ubuntu.

@Mesabloo
Copy link
Owner

Thanks!

I'll wait a bit before merging this PR.
Given the current state of the wcwidth package on Hackage, I'd rather not include CI if it will fail for at least Windows at the moment.
Hopefully we get an update soon...otherwise I'll resort to including wcwidth inside the library (in a hidden module probably) and we'll see after that!

@Mesabloo Mesabloo added the enhancement New feature or request label Oct 13, 2022
@BinderDavid
Copy link
Author

Our temporary fix is to add the wcwidth Github repo as an extra dependency in our stack configuration: duo-lang/duo-lang#491

We hope to remove that workaround once wcwidth has been fixed upstream and a new version uploaded to Hackage.

@Mesabloo
Copy link
Owner

I think it might be a good idea to do the same here, and also to document it (until a new version of wcwidth is uploaded to Hackage) for other users.

However, a quick question before: from what I see in the stack.yaml file, you are using the latest commit made to wcwidth. Does it work with spaces? As far as we have discussed in #11 (comment), the fix should be pretty easy but I don't see it fixed there.

@Mesabloo
Copy link
Owner

Let's merge this, but we know that currently, Windows support for wcwidth (at least the officially published package) is a bit lacking.
Would you mind commenting out windows-latest in the build matrix? I'll merge this right after. Or a better solution would be to add the dependency in the stack.yaml file?
Either way, I'll add a warning in the README once this is merged.
Thanks!

@BinderDavid
Copy link
Author

Sorry, it took me quite a while to come back to this. I have written to the maintainer of wcwidth and have asked to take over maintenance of the wcwidth package on Hackage. If that works out then I will be able to release a new version of wcwidth on Hackage, and properly fix diagnose on Windows.

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

Successfully merging this pull request may close these issues.

2 participants