replace s3pository with nodebin #540
Conversation
hone
force-pushed
the
nodebin
branch
4 times, most recently
from
f456e0f
to
24650ff
Compare
|
Awesome start, tests are failing. Looks like due to rate limiting. Restarting tests. |
hone
force-pushed
the
nodebin
branch
17 times, most recently
from
6e62447
to
5b4b845
Compare
upgrade node to latest Node LTS for Rails 5.1 s3pository is being replaced with nodebin can't push to cedar-10 stack anymore, so don't need "legacy" node 0.12 is ancient at this point, we should pin to LTS now
|
added |
lib/language_pack/helpers/nodebin.rb
Outdated
| @@ -0,0 +1,24 @@ | |||
| require 'json' | |||
|
|
|||
| class LanguagePack::Nodebin | |||
There was a problem hiding this comment.
The other helpers are name-spaced LanguagePack::Helpers
| end | ||
|
|
||
| def self.node_lts | ||
| node("latest?range=6.x") |
There was a problem hiding this comment.
Will we need to manually rev this for future versions of node?
There was a problem hiding this comment.
yeah, I'm asking @hunterloftis for a /lts route or param. It'd be nice not having to keep up to date with it. I think he's trying to keep changes to a minimum until after he ships it to prod for now. /cc @jmorrell
There was a problem hiding this comment.
I'm down with the idea & if @jmorrell wants to implement 👍 but I wouldn't sweat the maintenance burden too much (once a year updates).
There was a problem hiding this comment.
I'd be happy to submit a PR too if you're down with it :) I just imagine if say the Go buildpack needs to start bundling yarn + node, it would be nice to have a path for it.
| node_bin = "#{binary_path}/bin/node" | ||
|
|
||
| Dir.chdir(dir) do | ||
| @fetcher.fetch_untar(@url, node_bin) |
There was a problem hiding this comment.
Why is the mktmpdir dance needed here?
There was a problem hiding this comment.
it's not looking at it now.
lib/language_pack/ruby.rb
Outdated
| bin_dir = "bin" | ||
| FileUtils.mkdir_p bin_dir | ||
| Dir.chdir(bin_dir) do |dir| | ||
| if name.match(/^node\-/) | ||
| @node_installer.install | ||
| node_bin_path = File.absolute_path(".") | ||
| ENV["PATH"] = "#{ENV["PATH"]}:#{node_bin_path}" |
There was a problem hiding this comment.
It's weird that this is needed. bin/ should already be on the path.
There was a problem hiding this comment.
we need this b/c we need the absolute path during yarn install. some node modules using node-gyp will change the CWD and try to execute node and it won't be there. i'll add a comment.
| Hatchet::Runner.new("rails51_webpacker").deploy do |app, heroku| | ||
| expect(app.output).to include("Installing yarn") | ||
| expect(app.output).to include("yarn install") | ||
| expect(app.run("rails -v")).to match("") |
There was a problem hiding this comment.
We supposed to be expecting a rails version here?
Maybe we should check status of yarn -v?
There was a problem hiding this comment.
sure. I just copied the rails5 test I think.
upgrade node to latest Node LTS for Rails 5.1
s3pository is being replaced with nodebin
can't push to cedar-10 stack anymore, so don't need "legacy"
node 0.12 is ancient at this point, we should pin to LTS now
/cc @hunterloftis