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
Locked platform dependencies inside case block in gemspec #454
Conversation
@@ -1,4 +1,6 @@ | |||
# This is just so stubs work | |||
return unless RUBY_PLATFORM === /darwin/i |
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.
Style/CaseEquality: Avoid the use of the case equality operator ===.
s.add_dependency 'rb-fsevent', '~> 0.10', '>= 0.10.3' | ||
when /linux/i | ||
s.add_dependency 'rb-inotify', '~> 0.9', '>= 0.9.10' | ||
end |
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.
I don't think this is going to work since .gemspec
are evaluated at the time the gem is build (i.e. only the dependencies matching the machine that builds it will be listed). See https://stackoverflow.com/a/41694151/304055.
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.
I read it but requirements on building a gem depending on the ENV variable is not the same case, because the dependency here is on platform i.e. exactly build/install env.
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.
I think the problem is the same, the .gemspec
is evaluated at build time, thus one the developer's machine. For instance, if I were to release a new version, the built gem would depend on rb-fsevent
only since I'm on macOS, and people on Linux would need to manually install rb-inotify
.
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.
may be it should be ok, and just be explicitly described in manual
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.
Hmm, I think that was discussed before and we decided that the best trade-off was to keep both dependencies in the .gemspec
. I'm going to close this PR since we cannot accept it. Thanks for contributing!
No description provided.