Conversation
|
Another thing that needs fixed is merging coverage reports from the two parts. After some googling, the best I found was switching to codecov.io instead, which does automerging of ALL reports sent in, so it could also ingest the reports from non-default rubies, and windows into a single result. I've played around with it a little bit at another project this morning: https://codecov.io/gh/DavidS/puppet-resource_api/commit/53af7f92b209d07bb1cac838bd9d33e06906d103/build |
1 similar comment
bc882e6 to
480028b
Compare
4 similar comments
3 similar comments
Codecov Report
@@ Coverage Diff @@
## dev #127 +/- ##
======================================
Coverage ? 83.13%
======================================
Files ? 29
Lines ? 1755
Branches ? 0
======================================
Hits ? 1459
Misses ? 296
Partials ? 0Continue to review full report at Codecov.
|
|
Ah. Now it makes more sense what you wanted to do. Admittedly, I only mostly followed what your plan was when you opened the issue. I'll take a better look later this week but, at a glance, it'd be nice if the two top level project folders were named the same as their gem ( Also, please base the PR off of the development branch ( I appreciate all of the effort! |
aebb493 to
e7ea7a7
Compare
|
I've renamed the top-level directories, and rebased everything. That was the easy part. I also tried to rename the ChildProcess module in core. This would split the exiting public API on the ChildProcess module across namespaces, which will have a big impact on downstream users. While I tried it out, I also found pieces where it's not really clear that the way it's setup currently makes sense in a multi-gem world. E.g. https://github.com/enkessler/childprocess/blob/master/lib/childprocess.rb#L203-L205 might need more work. Looking at that, I wonder if splitting it even more into |
2 similar comments
The final goal here is a separate childprocesscore gem that contains all ruby-only code, specifically not FFI). posix_spawn, windows support, the related tooling, and the ffi dependency stays in the main gem. This way people used to the existing ways can keep trucking along without disturbance, while people who want to avoid FFI can depend on just childprocesscore. This is only the first patch in the series, to keep the code patches cleaner.
This could be done better by moving more of the machinery to main, but it'll serve for now.
e7ea7a7 to
59fe72a
Compare
9 similar comments
|
Something weird is still ongoing with the codcov reports (see https://codecov.io/gh/DavidS/childprocess/compare/aebb49369bcebc401f0c59af8e336d47a18944c1...59fe72a87c0264ab724f21c1f27d56971fa25434/changes), but I'll chalk that up to weirdness around the changes in the repos. And appveyor failed on SSL when trying to upload reports. Everything else shows up on https://codecov.io/gh/DavidS/childprocess/commit/59fe72a87c0264ab724f21c1f27d56971fa25434/build |
|
@DavidS The more I think about it, the more that I just want the correct version to be downloaded automatically. If they are on Windows, then they get the windows version that will have FFI as a dependency and if they are not then they get the other version. This looks relevant: http://guides.rubygems.org/specification-reference/#platform= It would require us to build two versions of the gem but that can be a Rake task and the only difference would be the dependencies in the gemspec and a conditional around the FFI parts in the library code. |
|
@enkessler something like this: https://github.com/puppetlabs/puppet-resource_api/blob/master/ext/mkrf_conf.rb ? |
|
Possibly, yes. I take it that that is a script that is kicked off during the gem installation process that conditionally triggers additional gem installations? And it looks like you are already using it, so it presumably works. Out of curiosity, what does the console output look like for the two different installation paths? Also, I see that this was one of your first suggestions before but I glossed over it and focused on the alternative because you said
Can you elaborate on that? What do you mean by 'available'? Won't it be downloaded the same as any other gem just, you know, conditionally? |
|
The code I linked installs a package conditionally on a platform. If we choose to "install FFI only on windows", FFI would not be available by default for |
|
Closing this in favour of #132 |
This splits off all of the pure ruby code into a childprocesscore gem that doesn't depend on FFI, thereby allowing users to trade off between full-featured, and always-deployable by depending on the right gem. It is even possible for grandchildren to add the childprocess and FFI gems on top of something using only childprocesscore to enable posix_spawn, or other platform support.
This still needs updates to the README, but I wanted to get early feedback on the details of the approach.
Fixes #124