Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
tl;dr
This PR is only concerned about getting green builds, not fixing the underlying
issues that caused it to go red in the first place. Proposed solutions are being
described below and are up for discussion.
node version
nvm install node
will install the latest node version, which is 12 asof writing. Because of a problem in the resolved node-sass version this
makes the builds fail.
To get green builds until a proper fix for node 12 is introduced we'll
install node 10, which is a LTS release, while 11 will be EOL in June
2019.
@rokumatsumoto is maybe working on a fix, but I think green builds,
especially for third party PRs are an important intermediate step.
#2077
sass/node-sass#2633
nodejs/nan#849
https://github.com/nodejs/Release
RuboCop version
RuboCop 0.69.0 dropped support for Ruby 2.2, making RuboCop fail when it
was installed since it pointed to the git repo instead of a specific
version.
Is there any reason why dev dependencies are declared in each and every Gemfile
instead of as a dev dependency inside the
.gemspec
file? I moved RuboCop tothe
.gemspec
to avoid duplication.https://github.com/rubocop-hq/rubocop/blob/v0.69.0/CHANGELOG.md#changes
Testing with
ruby-head
These might – currently – not be related to Ruby itself, but to the fact that
bundler 2.1.0.pre.1 was installed, which is as of writing not supported by
webpacker (
~> 1.12
is used).But this is to get the CI builds green again.
Proposed solutions going forward
Use
.gemspec
file moreI don't know if there's a specific reason to have so many development
dependencies inside Gemfiles instead of the
.gemspec
file, but I think itwould make sense to have more of them in
.gemspec
to avoid duplication.Relax bundler version constraint
Currently bundler is locked to a version
< 2
, see here. I think it would makesense to allow bundler 2 as well since it's the current stable release. There
might be problems with CI, but it can be dealt with, especially once Rails 6 is
released, dropping Rails 4.2 support might make sense for webpacker as well. And
Rails 4.2 is the only Rails release explicitly wanting bundler 1.x.
Node.js versions
I think it could make sense to drop Node.js 6 support since it will be EOL very
soon (probably around the time Rails 6 is released, so it would make sense to
have a major release of webpacker with a little cleanup.
That being said, we could consider testing with all supported Node.js versions,
as we've seen always using the latest version might lead to an unstable CI
system. It would probably make test runs much longer if we test all LTS
versions, but it's something to consider. At least testing with a specific LTS
release and the latest version (think
ruby-head
).Dropping Ruby 2.2 support
I'm not sure if this is an options since Rails 5 supports Ruby 2.2.2 and higher,
but since the ecosystem (see RuboCop in this PR) is moving forward, it might
make sense.
Note that testing with Ruby 2.2 was removed over a year ago.