Skip to content
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

slim-lintのルール「Layout/LineLength」を適用する #7345

Merged
merged 5 commits into from
Mar 11, 2024

Conversation

a-terumoto-gs
Copy link
Contributor

@a-terumoto-gs a-terumoto-gs commented Feb 9, 2024

Issue

概要

slim-lintが指摘するルールで無視する設定になっているもののうち、Layout/LineLengthを有効化する

Layout/LineLengthとは

一行の長さが長すぎないかを確認するルール。160文字を越えると指摘がはいるようになっている。
Layout/LineLength - Rubocop Docs

# bad
{foo: "0000000000", bar: "0000000000", baz: "0000000000"}

# good
{foo: "0000000000",
bar: "0000000000", baz: "0000000000"}

# good (with recommended cops enabled)
{
  foo: "0000000000",
  bar: "0000000000",
  baz: "0000000000",
}

このルールを適用したところ、以下の30の指摘が付いたため修正を行った。
(長すぎるためたたんでおります)

app/views/activity_mailer/assigned_as_checker.html.slim:1 [W] RuboCop: Layout/LineLength: Line is too long. [174/160]
app/views/activity_mailer/came_answer.html.slim:1 [W] RuboCop: Layout/LineLength: Line is too long. [177/160]
app/views/activity_mailer/consecutive_sad_report.html.slim:1 [W] RuboCop: Layout/LineLength: Line is too long. [187/160]
app/views/activity_mailer/create_page.html.slim:1 [W] RuboCop: Layout/LineLength: Line is too long. [169/160]
app/views/activity_mailer/hibernated.html.slim:1 [W] RuboCop: Layout/LineLength: Line is too long. [167/160]
app/views/activity_mailer/mentioned.html.slim:1 [W] RuboCop: Layout/LineLength: Line is too long. [162/160]
app/views/activity_mailer/no_correct_answer.html.slim:1 [W] RuboCop: Layout/LineLength: Line is too long. [196/160]
app/views/activity_mailer/product_update.html.slim:1 [W] RuboCop: Layout/LineLength: Line is too long. [167/160]
app/views/activity_mailer/submitted.html.slim:1 [W] RuboCop: Layout/LineLength: Line is too long. [167/160]
app/views/application/_required_field.html.slim:24 [W] RuboCop: Layout/LineLength: Line is too long. [168/160]
app/views/application/_tags_input.slim:1 [W] RuboCop: Layout/LineLength: Line is too long. [212/160]
app/views/articles/_recent_articles.html.slim:14 [W] RuboCop: Layout/LineLength: Line is too long. [166/160]
app/views/events/_participation.html.slim:17 [W] RuboCop: Layout/LineLength: Line is too long. [161/160]
app/views/events/_participation.html.slim:36 [W] RuboCop: Layout/LineLength: Line is too long. [164/160]
app/views/notification_mailer/trainee_report.html.slim:1 [W] RuboCop: Layout/LineLength: Line is too long. [175/160]
app/views/pages/_doc_header.html.slim:79 [W] RuboCop: Layout/LineLength: Line is too long. [187/160]
app/views/practices/questions/index.html.slim:12 [W] RuboCop: Layout/LineLength: Line is too long. [165/160]
app/views/products/unchecked/index.html.slim:18 [W] RuboCop: Layout/LineLength: Line is too long. [231/160]
app/views/questions/_tabs.html.slim:5 [W] RuboCop: Layout/LineLength: Line is too long. [167/160]
app/views/questions/_tabs.html.slim:12 [W] RuboCop: Layout/LineLength: Line is too long. [164/160]
app/views/questions/_tabs.html.slim:14 [W] RuboCop: Layout/LineLength: Line is too long. [202/160]
app/views/regular_events/_participation.html.slim:8 [W] RuboCop: Layout/LineLength: Line is too long. [183/160]
app/views/regular_events/_participation.html.slim:14 [W] RuboCop: Layout/LineLength: Line is too long. [181/160]
app/views/reports/_learning_time_fields.html.slim:3 [W] RuboCop: Layout/LineLength: Line is too long. [165/160]
app/views/survey_questions/edit.html.slim:27 [W] RuboCop: Layout/LineLength: Line is too long. [213/160]
app/views/survey_questions/new.html.slim:27 [W] RuboCop: Layout/LineLength: Line is too long. [213/160]
app/views/surveys/show.html.slim:102 [W] RuboCop: Layout/LineLength: Line is too long. [165/160]
app/views/users/show.html.slim:48 [W] RuboCop: Layout/LineLength: Line is too long. [215/160]
app/views/users/show.html.slim:96 [W] RuboCop: Layout/LineLength: Line is too long. [178/160]
app/views/welcome/_mentors.html.slim:7 [W] RuboCop: Layout/LineLength: Line is too long. [162/160]

指摘が付いていた箇所のコードは長くて読みにくいものになっていたため、可読性がよくなるような箇所のカンマで改行をいれ対応しました

確認したファイルの中で、指摘がついていない箇所であっても、指摘が付いている箇所と同じ構造になっていて、1行が長めで可読性が低かった箇所は、形式を合わせておいた方がのちのメンテナンスの際など見やすそうだと考えたため適宜改行をいれました

よって修正箇所は指摘されていた箇所以外も含まれ30か所以上になっています

基本的に改行のみで対応したのですが、以下2ファイルに関しては、改行+バックスラッシュ追加を行いました
app/views/reports/_learning_time_fields.html.slimの文字列部分
app/views/welcome/_mentors.html.slimのifの条件節部分

slim lintのきまりとして、一続きのコードを改行を挟んで記述するさいは、バックスラッシュが必要なため、上記2ファイルはバックスラッシュも追加する形で対応しました

変更確認方法

  1. feature/apply-layout-line-lengthをローカルに取り込む
  2. bundle exec slim-lint app/views -c config/slim_lint.ymlを実行する
  3. 指摘がないことを確認する

Screenshot

機能の変更はないので、見た目の変化はありません。

@a-terumoto-gs a-terumoto-gs self-assigned this Feb 9, 2024
@a-terumoto-gs a-terumoto-gs requested a review from ham-cap February 9, 2024 04:45
@a-terumoto-gs a-terumoto-gs marked this pull request as ready for review February 9, 2024 04:46
@a-terumoto-gs
Copy link
Contributor Author

@ham-cap
お疲れ様です。
急ぎではありませんので、お手すきの際にこちらのレビューをお願いしてもよろしいでしょうか?
よろしくお願いいたしますm(_ _)m

@a-terumoto-gs
Copy link
Contributor Author

@ham-cap
お疲れ様です。再度メンション失礼いたします。
こちらのレビュー対応いただくの難しい状況でしょうか?
忙しい等あれば他の方にお願いすること可能なので、その旨お伝えいただけると助かります。

よろしくお願いいたしますm(__)m

@a-terumoto-gs
Copy link
Contributor Author

@ham-cap
お疲れ様です。
このissueレビューはほかの方にお願いさせてもらおうと思います。
お騒がせいたしましたm(__)m

@a-terumoto-gs
Copy link
Contributor Author

@unikounio
お疲れ様です。
急ぎではありませんので、お手すきの際にこちらのレビューをお願いしてもよろしいでしょうか?
よろしくお願いいたしますm(_ _)m

@a-terumoto-gs a-terumoto-gs requested review from unikounio and removed request for ham-cap March 6, 2024 07:20
@unikounio
Copy link
Contributor

@a-terumoto-gs さん
承知しました!
今週中にはレビューをお返しできるようにしたいと思います😄
よろしくお願いいたします。

Copy link
Contributor

@unikounio unikounio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

確認させていただきました!

ご提示いただいた変更確認方法に従い、slim-lintの指摘がないことを確認できました。
また、変更箇所のインデントについても、私が見る限り適切に行われていました。
よって、私からはApproveとさせていただきます。

概要欄でのご説明が丁寧で助かりました~😄
引き続き一緒にがんばりましょう~💪

@a-terumoto-gs a-terumoto-gs requested a review from komagata March 7, 2024 08:12
@a-terumoto-gs
Copy link
Contributor Author

@unikounio
ありがとうございました~!
変更加えたファイルがたくさんあったにもかかわらず、素早い対応に感謝します(^^)/

@komagata
お疲れ様です!
メンバーの方にapproveいただいたのでレビューお願いいたします!

@ham-cap
Copy link
Contributor

ham-cap commented Mar 8, 2024

@a-terumoto-gs
お疲れ様です。
返信ができておらず大変申し訳ございませんでした🙇
仕事が忙しく全く時間が取れなかったためメンションも見落としてしまっていました💦
既に他の方にご依頼いただいているかと思いますが、取り急ぎお詫びのご連絡です🙇

@komagata
Copy link
Member

komagata commented Mar 9, 2024

@a-terumoto-gs コンフリクトの解消をお願いします〜

@a-terumoto-gs a-terumoto-gs force-pushed the feature/apply-layout-line-length branch 2 times, most recently from 038eab3 to 31a8082 Compare March 11, 2024 08:10
@a-terumoto-gs
Copy link
Contributor Author

a-terumoto-gs commented Mar 11, 2024

@komagata
コンフリクト解消し、mainブランチの変更を取り込んだことで生じていた指摘も解消するように修正しました。
確認よろしくお願いいたしますm(__)

Copy link
Member

@komagata komagata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

確認させて頂きました。OKです〜🙆‍♂️

@komagata komagata merged commit 163517b into main Mar 11, 2024
2 checks passed
@komagata komagata deleted the feature/apply-layout-line-length branch March 11, 2024 18:36
@github-actions github-actions bot mentioned this pull request Mar 11, 2024
29 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants