モデル生成時にreferences/belongs_toを指定した場合に、モデルのマイグレーションでindexを別に作らせない#11
Draft
mk2 wants to merge 24 commits into
Draft
モデル生成時にreferences/belongs_toを指定した場合に、モデルのマイグレーションでindexを別に作らせない#11mk2 wants to merge 24 commits into
mk2 wants to merge 24 commits into
Conversation
* add postgres 10.21, 11.16, 12.11, 13.7, 14.3 * use versions existing on hub.docker.com (10.20, 11.15, 12.10, 13.6, 14.2) * Upgrade ci postgres version from 9.5 to 10 (drop 10.20, 11.15, 12.10, 13.6, 14.2) Co-authored-by: Ko Nagase <nagase@georepublic.co.jp>
* Add VSCode devcontainer用の設定を追加 * Add docker-compose.yml * Move Dockerfile-for-redmine-dev-mirror to .devcontainer/Dockerfile * Fix Rubyのバージョン変更の手順 * Update README.md * Update DockerfileでADDしなくなったことによって機能しなくなっていたコードを削除
* Adjust docker-compose.yml MySQL settings and expose PostgreSQL/MySQL ports * Adjust SQLTools MySQL driver and connections
mk2
commented
Jun 30, 2023
| class_option :migration, :type => :boolean, :default => true | ||
| class_option :timestamps, :type => :boolean | ||
| class_option :parent, :type => :string, :desc => "The parent class for the generated model" | ||
| class_option :indexes, :type => :boolean, :default => true, :desc => "Add indexes for references and belongs_to columns" |
Contributor
Author
There was a problem hiding this comment.
これを残す方向での修正も検討したのですが、下記の想定を行った結果消すことにしました。
foreign_key: trueを指定せずにマイグレーションを生成しないといけない- 別途
add_indexを書きたいのはカスタマイズしたindexを貼りたいからというのが大きいと思う、であればgeneratorは何もせず、自分で書いてもらう方が良い
kumojima
approved these changes
Jul 3, 2023
|
|
||
| def attributes_with_index | ||
| attributes.select {|a| a.has_index? || (a.reference? && options[:indexes])} | ||
| attributes.select {|a| a.has_index?} |
Contributor
There was a problem hiding this comment.
Suggested change
| attributes.select {|a| a.has_index?} | |
| attributes.select(&:has_index?) |
nishidayuya
approved these changes
Jul 4, 2023
nishidayuya
left a comment
nishidayuya
left a comment
There was a problem hiding this comment.
良いと思いました! 🎉 ご対応おつかれさまでした．
気づいた点は細かいことですのでApproveします．
|
|
||
| assert_match %r{t\.references :issue, foreign_key: true}, migration | ||
| assert_match %r{t\.belongs_to :project, foreign_key: true}, migration | ||
| assert_no_match %r{add_index}, path_names.first.read |
There was a problem hiding this comment.
細かいですが，以下で良さそうです．
Suggested change
| assert_no_match %r{add_index}, path_names.first.read | |
| assert_no_match %r{add_index}, migration |
telegib
approved these changes
Jul 14, 2023
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
9 participants
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.
lib/generators/redmine_plugin_model/templates/migration.rbについては、修正とは直接関係ないですが、明らかにerbファイルなのでリネームしています。