Merged
Conversation
Contributor
ippachi
commented
Mar 23, 2025
- 違反メソッドの複数検知に対応
- publicメソッド以外を検知対象から除外
- 推奨メソッドの設定機能を追加
- 違反メソッドの複数検知に対応 - publicメソッド以外を検知対象から除外 - 推奨メソッドの設定機能を追加 close: #1
There was a problem hiding this comment.
Pull Request Overview
This PR improves the ControllerRecommendRestfulMethods cop by supporting the detection of multiple non-RESTful methods, limiting its checks to public methods only, and introducing configurable recommended methods via the configuration file.
- Updated .rubocop.yml to add inherit_mode for merging and exclude Metrics/BlockLength for specs.
- Extended tests in the spec file to verify detection of multiple offenses and ensure non-public methods are excluded.
- Modified the cop implementation to iterate over all action declarations and support configuration-driven recommended methods, with corresponding updates to the default configuration in config/default.yml.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| .rubocop.yml | Added inherit_mode merge setting and Metrics/BlockLength exclusion for specs. |
| spec/rubocop/cop/dobado_architecture/controller_recommend_restful_methods_spec.rb | Introduced new test cases to validate multiple offense detection, public-only filtering, and custom configuration. |
| lib/rubocop/cop/dobado_architecture/controller_recommend_restful_methods.rb | Refactored the cop to support multiple detections and configurable recommended methods; removed VisibilityHelp inclusion. |
| config/default.yml | Added default RecommendedMethods configuration used by the cop. |
Comments suppressed due to low confidence (1)
lib/rubocop/cop/dobado_architecture/controller_recommend_restful_methods.rb:40
- The call to non_public?(action) may fail because the VisibilityHelp module, which likely provided this method, has been removed. Consider reintroducing the method or implementing the public check logic directly.
next if non_public?(action)
perugo
approved these changes
Mar 25, 2025
perugo
left a comment
There was a problem hiding this comment.
実際に手元でテストを作ってみて良い感じでした。
モジュール下のクラスに対してもテストが適用されてました 🙆♂️
module XXX
class class UsersController < ApplicationController
|
本PRとは関係ないですが、 gitignoreに |
Contributor
Author
|
元からなかった |
Contributor
Author
|
@AT274 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.