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

fix CT regex https://github.com/nitely/nim-regex/issues/4 #7

Merged
merged 2 commits into from Jul 3, 2019

Conversation

timotheecour
Copy link
Contributor

@timotheecour timotheecour commented Jul 3, 2019

/cc @nitely please don't forget to create new git tag for both nim-regex and nim-unicodedb after merging!

note

CI fails for 0.18.0 (works for 0.19); although i could try to make it work for 0.18, how about dropping support for 0.18 (quite old now) and adding a CI for 0.20 instead?
https://travis-ci.org/nitely/nim-unicodedb/jobs/553541005

/usr/src/app/src/unicodedb/types.nim(38, 28) Error: type expected

src/unicodedb/types.nim Outdated Show resolved Hide resolved
@nitely
Copy link
Owner

nitely commented Jul 3, 2019

This is awful and clever and I like it. Make tests green and I'll merge it.

@timotheecour
Copy link
Contributor Author

green!

@nitely nitely merged commit 59336cd into nitely:master Jul 3, 2019
@nitely
Copy link
Owner

nitely commented Jul 3, 2019

Thanks!

PS. There is no need to release a new regex version, I think.

@timotheecour timotheecour deleted the pr_fix_regex_ct branch July 3, 2019 19:35
@timotheecour
Copy link
Contributor Author

@nitely
nitely/nim-regex#4 is likely root-caused by nim-lang/Nim#12727 ; and nim-lang/Nim#12777 might make this change obsolete; might be worth making this hack conditional:
when (NimMajor, NimMinor) < (x, y): ...

@nitely
Copy link
Owner

nitely commented Dec 3, 2019

@timotheecour ah, yes. I'll keep an eye on that PR, and test later. Thanks!

@timotheecour
Copy link
Contributor Author

that PR nim-lang/Nim#12777 just got merged

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

2 participants