-
-
Notifications
You must be signed in to change notification settings - Fork 904
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
Darwin native gems #2073
Darwin native gems #2073
Conversation
Code Climate has analyzed commit ade7ec1 and detected 4 issues on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 100.0% (80% is the threshold). This pull request will bring the total coverage in the repository to 94.3%. View more on Code Climate. |
where the gem platform name is different from RUBY_PLATFORM. - call the gem task correctly - have `nokogiri -v` display Gem::Platform.local
so all our cross-compilation and native gem logic is in one place
- update CrossRuby to recognize it - improve the parsing code for .cross_rubies - only create rake-compiler-docker rake tasks for windows and linux
b81d9a6
to
bbe0132
Compare
bbe0132
to
fc72c85
Compare
✅ Build nokogiri 1.0.660 completed (commit 0f04d62133 by @flavorjones) |
✅ Build nokogiri 1.0.664 completed (commit 3e6f90c3e7 by @flavorjones) |
and make sure we gitignore the .bundle files
and we need to optionally allow the objc libraries
9f19c70
to
ade7ec1
Compare
end | ||
end | ||
|
||
def dlls | ||
def allowed_dlls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Method allowed_dlls
has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.
end | ||
end | ||
|
||
def dlls | ||
def allowed_dlls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Method allowed_dlls
has 30 lines of code (exceeds 25 allowed). Consider refactoring.
"/usr/lib/libSystem.B.dylib", | ||
"/usr/lib/liblzma.5.dylib", | ||
"/usr/lib/libobjc.A.dylib", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@larskanis @tenderlove I could use a sanity check on these remaining references. This seems reasonable to me. I'll probably just ship an RC and see if we get feedback, but wanted to highlight that I made a decision with incomplete information here.
I'm going to merge this so I can ship an RC and get feedback. Would still love to hear anybody's feedback about this! |
What problem is this PR intended to solve?
Proposed work for #2063 -- precompiled native gems for OSX/Darwin.
Have you included adequate test coverage?
Needs:
Does this change affect the C or the Java implementations?
No functional changes -- only developer changes in how we build gems.