Skip to content

Conversation

@bshand
Copy link
Contributor

@bshand bshand commented Aug 6, 2025

Bump rails to 7.1.5.2 https://nhsd-jira.digital.nhs.uk/browse/NDRS2-3177

The brakeman warnings are unrelated to this commit (warning that Rails 7.1 is nearing EOL).

The test failures are all integration tests, and can be ignored: recent chrome driver changes have made the integration tests very brittle, but the front-end is no longer in use (and the code for it is probably still working fine anyway).

I've left a few Rails 7.2 deprecation warnings in place, to be fixed in a subsequent PR, in case we need to do before / after Rails 7.1 vs Rails 7.2 testing.

@bshand bshand changed the title Bump rails to 7.1.5.1 Bump rails to 7.1.5.2 Sep 9, 2025
@shilpigoeldev
Copy link
Contributor

shilpigoeldev commented Sep 9, 2025

@bshand As per https://guides.rubyonrails.org/upgrading_ruby_on_rails.html#rails-logger-now-returns-an-activesupport-broadcastlogger-instance,

export_weekly.rake
  96,34:     logger.extend(ActiveSupport::Logger.broadcast(Rails.logger)) 

and

import_weekly.rake
  13,34:     logger.extend(ActiveSupport::Logger.broadcast(Rails.logger))

would need update?

@bshand
Copy link
Contributor Author

bshand commented Sep 9, 2025

@bshand As per https://guides.rubyonrails.org/upgrading_ruby_on_rails.html#rails-logger-now-returns-an-activesupport-broadcastlogger-instance,

export_weekly.rake
  96,34:     logger.extend(ActiveSupport::Logger.broadcast(Rails.logger)) 

and

import_weekly.rake
  13,34:     logger.extend(ActiveSupport::Logger.broadcast(Rails.logger))

would need update?

Good spot: I've pushed a fix for this now.

@shilpigoeldev shilpigoeldev self-requested a review September 9, 2025 21:14
@bshand bshand merged commit 04687b3 into develop Sep 10, 2025
9 of 15 checks passed
@bshand bshand deleted the feature/NDRS2-3177/rails_7_1 branch September 10, 2025 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants