-
Notifications
You must be signed in to change notification settings - Fork 6
Worker Rails 7.2 #240
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
Worker Rails 7.2 #240
Conversation
Code quality scoreLovely, the code quality is unchanged for this PR 😊
|
Gemfile
Outdated
| # gem 'supplejack_common', path: '~/Dev/supplejack/gems/supplejack_common' | ||
| # gem 'supplejack_common', github: 'DigitalNZ/supplejack_common', branch: 'pm/upgrade' | ||
| gem 'supplejack_common', github: 'DigitalNZ/supplejack_common', tag: 'v3.0.2' | ||
| gem 'supplejack_common', github: 'DigitalNZ/supplejack_common', branch: 'tw/rails-7-2' |
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.
Will this need changed before you merge? Do the other 2 commented out lines need removed?
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.
Yep I'll need to update this once the supplejack common PR is merged. Good point about commented lines, we can probably remove these.
bin/setup
Outdated
| system! "bin/rails restart" | ||
|
|
||
| # puts "\n== Configuring puma-dev ==" | ||
| # system "ln -nfs #{APP_ROOT} ~/.puma-dev/#{APP_NAME}" |
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.
question: do you need to include this commented out or can you remove?
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.
good question, it was added by 'rails app:update' could probably remove
| end | ||
|
|
||
| it 'does not flush records if a harvest failure occured' do | ||
| it 'does not flush records if a harvest failure occurred' do |
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.
nice!
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.
👍
config/environments/production.rb
Outdated
| config.force_ssl = true | ||
|
|
||
| # Skip http-to-https redirect for the default health check endpoint. | ||
| # config.ssl_options = { redirect: { exclude: ->(request) { request.path == "/up" } } } |
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.
same as above
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.
good question, it was added by 'rails app:update' could probably remove
Worker Rails 7.2
As a Software Developer, I want to upgrade our Rails application from 7.1.5.1 to 7.2 so that we maintain support on the current Rails version
Acceptance Criteria: