Conversation
Also works for both systems in
|
29ab203
to
f515e02
Compare
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.
Looks good. I just don't understand how this is using .binstubs
config/deploy.rb
Outdated
# Default value for :linked_files is [] | ||
# set :linked_files, %w{config/database.yml} | ||
|
||
# Default value for linked_dirs is [] | ||
# set :linked_dirs, %w{bin log tmp/pids tmp/cache tmp/sockets vendor/bundle public/system} | ||
set :linked_dirs, %w(log run config/environments config/certs) | ||
set :linked_dirs, %w(.binstubs log run config/environments config/certs) |
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.
Why is .binstubs a linked directory?
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.
The bundle install creates this path as a utility and linking it here enables easy use of the command utils in there, http://bundler.io/v1.10/bundle_binstubs.html
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.
Why do we want to use binstubs on the server? What were we doing prior to this change? Is this something that needs to be part of this PR?
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.
What's wrong with it? It's useful in ssh sessions on the box.
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.
What's wrong with the default behavior?
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.
lol, this question tennis is not getting us anywhere; what's the objection to using .binstubs? Is it a blocker on this PR? Are you asking me to remove it or this PR is blocked? What do you want?
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 moving this to a different PR would be best. This isn't required to achieve the primary goal of the PR.
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.
Furthermore, it's suggested that binstubs be checked in if you're going to use them: https://github.com/rbenv/rbenv/wiki/Understanding-binstubs#bundler-generated-binstubs
You are encouraged to check these binstubs in the project's version control so your colleagues might benefit from them.
If they're linked directories, that implies they aren't in version control.
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.
OK, I'll try to separate this out (and better check there are no cron or other things that depend on having access to this bin path).
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.
The default is set :bundle_binstubs, -> { shared_path.join('bin') }
and this project already has a bin
directory, so hopefully we have no bin-file name conflicts (probably not).
d4376d1
to
eb632de
Compare
c850f27
to
df334a9
Compare
Partial solution for #33 - it works and it's deployed on
sul-sdr-services-dev.stanford.edu
along with the current shared_configs for that system, which seems to be OK in rescue, seee.g.