Complete rubocop todo #263
Conversation
| else | ||
| # This gem supports Rails 5 or later | ||
| module InertiaRails | ||
| module InertiaDebugExceptions |
There was a problem hiding this comment.
Dropped the patch for Rails < 5.1 since Inertia Rails requires Rails 6+
There was a problem hiding this comment.
Pull Request Overview
This PR removes the RuboCop todo configuration and fixes all outstanding RuboCop offenses in the codebase. The changes bring the code into compliance with the project's style guidelines by addressing formatting, syntax, and naming issues across numerous Ruby files.
- Adds frozen_string_literal comments to all Ruby files
- Fixes string literal consistency (single quotes preferred)
- Corrects spacing, indentation, and hash syntax issues
Reviewed Changes
Copilot reviewed 71 out of 78 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| .rubocop_todo.yml | Removed entire RuboCop todo configuration file |
| .rubocop.yml | Removed inheritance from rubocop_todo.yml |
| spec/support/shared_examples.rb | Added frozen_string_literal comment |
| spec/rails_helper.rb | Fixed string literals and file path expansion |
| spec/inertia/rspec_helper_spec.rb | Corrected hash spacing, block syntax, and string literals |
| spec/inertia/response_spec.rb | Fixed string literal consistency |
| spec/inertia/request_spec.rb | Improved hash syntax and block formatting |
| spec/inertia/rendering_spec.rb | Fixed line length issues with array formatting |
| spec/inertia/rails_mimic_spec.rb | Cleaned up hash syntax and whitespace |
| lib files | Added frozen_string_literal and fixed various style issues |
| spec/dummy files | Added frozen_string_literal and standardized string literals |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| expect(JSON.parse(response.body)['props'].symbolize_keys).to eq(show_only_props.merge(normal_shared_prop: 1)) | ||
|
|
||
| get conditional_share_index_path, headers: {'X-Inertia' => true} | ||
| expect(JSON.parse(response.body)['props'].deep_symbolize_keys).to eq(index_only_props.merge(normal_shared_prop: 1)) | ||
| get conditional_share_index_path, headers: { 'X-Inertia' => true } | ||
| expect(JSON.parse(response.body)['props'].symbolize_keys).to eq(index_only_props.merge(normal_shared_prop: 1)) | ||
|
|
||
| get conditional_share_edit_path, headers: { 'X-Inertia' => true } | ||
| expect(JSON.parse(response.body)['props'].deep_symbolize_keys).to eq(edit_only_props.merge(normal_shared_prop: 1)) | ||
| expect(JSON.parse(response.body)['props'].symbolize_keys).to eq(edit_only_props.merge(normal_shared_prop: 1)) |
There was a problem hiding this comment.
The method symbolize_keys should be deep_symbolize_keys to match the original functionality. The change from deep_symbolize_keys to symbolize_keys could cause test failures if nested hash keys need to be symbolized.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
there are no nested keys present in the test data
| '`lazy` is deprecated and will be removed in InertiaRails 4.0, use `optional` instead.' | ||
| ) | ||
|
|
||
| @value = value |
There was a problem hiding this comment.
The line @block = block was removed and replaced with super(&block), but the @block instance variable is still referenced in line 16 (@block&.call(params)). This will cause the call method to fail.
| @value = value | |
| @value = value | |
| @block = block |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
block is set in the base class
| module InertiaRails | ||
| module InertiaDebugExceptions | ||
| def render_for_browser_request(request, wrapper) |
There was a problem hiding this comment.
The patch consolidation removes separate version-specific patches but may break compatibility with older Rails versions. The code now assumes all Rails versions support render_for_browser_request method.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
we do not support Rails < 6, so it's fine
|
@skryukov what do you think about the copilot review? the keys change does jump out, was that not needed or is rubocop getting heavy handed? |
| '`lazy` is deprecated and will be removed in InertiaRails 4.0, use `optional` instead.' | ||
| ) | ||
|
|
||
| @value = value |
There was a problem hiding this comment.
block is set in the base class
| module InertiaRails | ||
| module InertiaDebugExceptions | ||
| def render_for_browser_request(request, wrapper) |
There was a problem hiding this comment.
we do not support Rails < 6, so it's fine
| expect(JSON.parse(response.body)['props'].symbolize_keys).to eq(show_only_props.merge(normal_shared_prop: 1)) | ||
|
|
||
| get conditional_share_index_path, headers: {'X-Inertia' => true} | ||
| expect(JSON.parse(response.body)['props'].deep_symbolize_keys).to eq(index_only_props.merge(normal_shared_prop: 1)) | ||
| get conditional_share_index_path, headers: { 'X-Inertia' => true } | ||
| expect(JSON.parse(response.body)['props'].symbolize_keys).to eq(index_only_props.merge(normal_shared_prop: 1)) | ||
|
|
||
| get conditional_share_edit_path, headers: { 'X-Inertia' => true } | ||
| expect(JSON.parse(response.body)['props'].deep_symbolize_keys).to eq(edit_only_props.merge(normal_shared_prop: 1)) | ||
| expect(JSON.parse(response.body)['props'].symbolize_keys).to eq(edit_only_props.merge(normal_shared_prop: 1)) |
There was a problem hiding this comment.
there are no nested keys present in the test data
|
Thanks for double checking! Good luck anyone with an open PR, here come spacing conflicts 😄 |
2 participants
TODO is something we should actually do. This PR fixes all RuboCop offenses.