Fix issues #10 and #12 #13
Conversation
| @@ -1,6 +1,9 @@ | |||
| source_paths.unshift(File.dirname(__FILE__)) | |||
|
|
|||
| run "mv app/assets app/frontend" | |||
| run "mkdir app/assets" | |||
| run "mv app/frontend/config app/assets/config" | |||
There was a problem hiding this comment.
This moves app/assets/config/manifest.js back to the place Sprockets requires it.
There was a problem hiding this comment.
Is that just Sprockets? I did find putting things in app/frontend to be by far the most brittle part of this template, so I wonder if we should just stick with Rails defaults.
There was a problem hiding this comment.
Sprockets hard codes this path. rails/sprockets-rails#369 is open to see if it can be made customisable. I take your point on the defaults. I'm loathe to give it up because the default feels gross but accepting the default might be the most pragmatic.
eoinkelly
force-pushed
the
fix-sprockets-install-error
branch
from
2945c53 to
3b15fd0
Compare
| # The block passed to "after_bundle" seems to run after `bundle install` | ||
| # but also after `webpacker:install` and after Rails has initialized the git | ||
| # repo | ||
| after_bundle do |
There was a problem hiding this comment.
This diff is a bit messy but basically I moved lots of commands inside the after_bundle hook. This allowed me to remove our git repo setup and our call to webpacker:install.
This reverts commit fc1c4f7.
| @@ -23,7 +23,7 @@ | |||
| # directory. Alternatively, in the individual `*_spec.rb` files, manually | |||
| # require only the support files necessary. | |||
| # | |||
| Dir[Rails.root.join("spec", "support", "**", "*.rb")].each { |f| require f } | |||
| Dir[Rails.root.join("spec/support/**/*.rb")].each { |f| require f } | |||
There was a problem hiding this comment.
rubocop has changed it's mind about separators because / is normalised by Ruby to be whatever the separator on the platform is (e.g. it becomes \ on Windows
All scripts are marked executable during the installation.
| route 'root "home#index"' | ||
| route %Q(mount Sidekiq::Web => "/sidekiq" # monitoring console\n) | ||
| route "require 'sidekiq/web'" | ||
| route <<-EO_ROUTES |
There was a problem hiding this comment.
I think that since this route definition includes three different routes:
- Lighthouse workaround
- Sidekiq/web
- root route
I think we should probably define these with three different route definitions, so that they are inherently three different routes
| @@ -1,5 +1,5 @@ | |||
| namespace :test do | |||
| task :coverage do | |||
| task coverage: :environment do | |||
There was a problem hiding this comment.
Does loading the environment influence the coverage at all? I was under the impression that simplecov needed to be loaded before the application to corrrectly calculate the coverage.
There was a problem hiding this comment.
Hmmm I did this because rubocop yelled at me about it. I take your point about coverage tho. I'll look into this.
| @@ -23,7 +23,7 @@ | |||
| # directory. Alternatively, in the individual `*_spec.rb` files, manually | |||
| # require only the support files necessary. | |||
| # | |||
| Dir[Rails.root.join("spec", "support", "**", "*.rb")].each { |f| require f } | |||
| Dir[Rails.root.join("spec/support/**/*.rb")].each { |f| require f } | |||
| @@ -1,6 +1,9 @@ | |||
| source_paths.unshift(File.dirname(__FILE__)) | |||
|
|
|||
| run "mv app/assets app/frontend" | |||
| run "mkdir app/assets" | |||
| run "mv app/frontend/config app/assets/config" | |||
There was a problem hiding this comment.
Is that just Sprockets? I did find putting things in app/frontend to be by far the most brittle part of this template, so I wonder if we should just stick with Rails defaults.
This fixes issues #10 and #12 in under Rails 6