-
Notifications
You must be signed in to change notification settings - Fork 919
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
Fix rails 7.1 deprecation warning #1038
Fix rails 7.1 deprecation warning #1038
Conversation
Is there anything we can do to fast track this PR merge? Our logs are flooded with deprecation warnings since we upgraded to Rails 7.1. This is a trivial fix with no breaking changes. |
c019e9f
to
74168e6
Compare
I have just rebased this with master. No changes were required. The file affected by this change hasn't been touched for 11 years. Would it be possible to trigger the merge checks so we can see if the tests pass across all the platform configurations? |
+1 to this |
+1 |
1 similar comment
+1 |
+1 |
+1 |
1 similar comment
+1 |
ping |
Dear anyone in the org, plz help @alindeman, @ghempton , @nz, @outoftime , @runlevel5 , @rywall, @vanstee |
@mark-young-atg would you mind taking a look on test failures here? |
This PR addresses the following Rails 7.1 deprecation warnings. ``` DEPRECATION WARNING: Bolding log text with a positional boolean is deprecated and will be removed in Rails 7.2. Use an option hash instead (eg. `color("my text", :red, bold: true)`) DEPRECATION WARNING: BOLD is deprecated! Use MODES[:bold] instead. ``` Below is the output from the original code and the code that replaces it. Where the original code provided `BOLD` as the second argument it was generating the bold terminal escape sequence, '\e[1m`, twice which was not achieving anything in addition to turning on bold. Original: color(event.payload[:path], BOLD, true) => "\e[1m\e[1mupdate\e[0m" Suggested: color(event.payload[:path], nil, bold: true) => "\e[1mupdate\e[0m" Original: color(name, GREEN, true) => "\e[1m\e[32mSOLR Request (21.4ms)\e[0m" Suggested: color(name, GREEN, bold: true) => "\e[1m\e[32mSOLR Request (21.4ms)\e[0m"
9f4d6a4
to
ed61f69
Compare
This reverts commit ed61f69.
This reverts commit 51913ff.
Tests where failing with the following NoMethodError: undefined method `password' for "http://solr.test/uri":String uri.password = "REDACTED" if uri.password ^^^^^^^^^ # ./vendor/bundle/ruby/3.1.0/gems/rsolr-2.6.0/lib/rsolr/error.rb:9:in `clean_uri' # ./vendor/bundle/ruby/3.1.0/gems/rsolr-2.6.0/lib/rsolr/error.rb:27:in `to_s' # ./lib/sunspot/session_proxy/retry_5xx_session_proxy.rb:29:in `message' # ./lib/sunspot/session_proxy/retry_5xx_session_proxy.rb:29:in `rescue in method_missing' # ./lib/sunspot/session_proxy/retry_5xx_session_proxy.rb:16:in `method_missing' # ./lib/sunspot/session_proxy/abstract_session_proxy.rb:11:in `index' # ./lib/sunspot.rb:190:in `index' # ./spec/api/session_proxy/retry_5xx_session_proxy_spec.rb:49:in `block (2 levels) in <top (required)>' # ------------------ # --- Caused by: --- # FakeRSolrErrorHttp: # A FakeRSolrErrorHttp for which `exception.message.to_s` raises NoMethodError. This was triggered by the test suite providing a String object to the rsolr gem where it is expecting a URI object. Ref: - lib/sunspot/session_proxy/retry_5xx_session_proxy.rb:29 and 33 - spec/api/session_proxy/retry_5xx_session_proxy_spec.rb:24 - https://github.com/rsolr/rsolr/blob/master/lib/rsolr/error.rb#L6-L13
@serggl I don't believe those errors were related to the changes introduced in this branch. I reverted back to master and still got the same failures. In the interest of trying to move things forward I've done some digging. From what I can see these test failures are caused by calls to |
This PR addresses the following Rails 7.1 deprecation warnings.
Below is the output from the original code and the code that replaces it. Where the original code provided
BOLD
as the second argument it was generating the bold terminal escape sequence ("\e[1m
") twice which was not achieving anything in addition to turning on bold.