Remove accept encoding gzip try#61
Conversation
There was a problem hiding this comment.
Code Review
This pull request removes the default 'Accept-Encoding: gzip' header from Google Cloud Storage service request options, and adds corresponding acceptance and unit tests to verify this behavior. The review feedback suggests improving the tests by avoiding the inline rescue modifier in favor of safe navigation, and using the more idiomatic must_be_nil expectation in Minitest.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| end | ||
|
|
||
| after do | ||
| @client.builder.delete(HeaderRecorder) rescue nil |
There was a problem hiding this comment.
According to the community Ruby Style Guide, using the inline rescue modifier is discouraged because it can silently swallow unexpected errors (such as NoMethodError on other parts of the chain) and make debugging difficult. Since the only potential reason for an exception here is if @client or @client.builder is nil, we can use the safe navigation operator (&.) instead.
@client&.builder&.delete(HeaderRecorder)References
- Avoid using rescue in its modifier form. Use safe navigation instead to handle potential nil values. (link)
|
|
||
| it "does not set Accept-Encoding gzip header by default" do | ||
| service = Google::Cloud::Storage::Service.new "my-project", default_credentials | ||
| _(service.service.request_options.header["Accept-Encoding"]).must_be :nil? |
There was a problem hiding this comment.
In Minitest, it is more idiomatic and readable to use the built-in must_be_nil expectation instead of must_be :nil?.
_(service.service.request_options.header["Accept-Encoding"]).must_be_nilReferences
- Use idiomatic Minitest expectations like must_be_nil instead of generic predicate checks. (link)
| end | ||
|
|
||
| before do | ||
| @fresh_storage = Google::Cloud::Storage.new |
There was a problem hiding this comment.
remove fresh_storage, add some name like storage like that
|
|
||
| before do | ||
| @fresh_storage = Google::Cloud::Storage.new | ||
| @client = @fresh_storage.service.service.client |
There was a problem hiding this comment.
why these many service.service.client
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
bundle exec rake ciin the gem subdirectory.closes: #<issue_number_goes_here>