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

Use new sys.thread package when possible on haxe 4 #3

Merged
merged 1 commit into from
Mar 28, 2019
Merged

Use new sys.thread package when possible on haxe 4 #3

merged 1 commit into from
Mar 28, 2019

Conversation

Aidan63
Copy link
Contributor

@Aidan63 Aidan63 commented Mar 22, 2019

Hello,
Haxe 4.0.0-rc.2 introduces the sys.thread package to replace the individual threading classes for interp, neko, cpp, hl, and java and I've gone through and made it use those classes when possible.
Hopefully I've found all the important conditional compilation sections, it's working for my use on haxe 4 but I don't use all the targets, hopefully the ci will catch any issues!

@sebthom
Copy link
Member

sebthom commented Mar 23, 2019

Hi Aidan,
thanks for your PR. Unfortunately your changes seem to break the library on Haxe 3.4.7 as you can see in the build logs at https://travis-ci.org/vegardit/haxe-concurrent/jobs/510055482

For the PR to be accepted please fix the syntax error and squash your commits.

Thank you!

Seb

simplified threads conditional compilation

Updated RLock to use sys.thread.Mutex when on haxe 4

Updated queue to use sys.thread.Deque when on haxe 4

Updated #if syntax to make older haxe versions happy

Simplified java and haxe version conditionals
@Aidan63
Copy link
Contributor Author

Aidan63 commented Mar 25, 2019

Hi (and sorry for the late reply), was meaning to sort out the issues over the weekend but forgot!

I've fixed up what appears to be the last few remaining issues and squashed everything down to a single commit, so once this CI round is done hopefully it should be good to go.

@sebthom sebthom merged commit ac18995 into vegardit:master Mar 28, 2019
@sebthom
Copy link
Member

sebthom commented Mar 28, 2019

Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants