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

FreeBSD little build fix proposal. #678

Merged
merged 2 commits into from
Feb 25, 2020

Conversation

devnexen
Copy link

No description provided.

Copy link
Member

@refi64 refi64 left a comment

Choose a reason for hiding this comment

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

One minor nit but LGTM overall, thanks!!

@@ -43,7 +43,11 @@
end

MRuby::Build.new do |conf|
toolchain :gcc
if RUBY_PLATFORM !~ /freebsd/
Copy link
Member

Choose a reason for hiding this comment

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

Nit: while we're doing this anyway, maybe also check for openbsd here?

@Quintus
Copy link
Member

Quintus commented Feb 19, 2020

Very nice. Thank you. I approve this, but it would be nice if someone could check if compiling works afterwards still on MacOS.

@Quintus
Copy link
Member

Quintus commented Feb 19, 2020

@refi64 Consider cherry-picking this into the release branch for 2.1.0. It should apply nearly as-is and does not change anything that could possible make problems I think.

@devnexen
Copy link
Author

Side note: would be nice to update the mruby dependency at some point. this https://github.com/mruby/mruby/blob/1c09046c13fc0a763583c6da0f1d350f6c41c4ca/mrbgems/mruby-compiler/core/keywords probe signatures are not correct (unsigned int instead of size_t) and has been fixed since.

@Quintus
Copy link
Member

Quintus commented Feb 25, 2020

Thanks for the heads-up. I will take care of mruby updates in the devel branch later on. The release-2.1.0 branch should not see such changes anymore.

@Quintus Quintus merged commit 367e090 into Secretchronicles:devel Feb 25, 2020
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