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

管理画面の企業一覧の企業名をリンクに置換 #4737

Merged
merged 1 commit into from
May 8, 2022

Conversation

isshi-hasegawa
Copy link
Contributor

@isshi-hasegawa isshi-hasegawa commented Apr 29, 2022

Issue

管理画面の企業一覧の企業名をリンクに置き換えました。
リンクに指定している URL は/companies/XXXXX で、 admin でなくても見れる会社個別ページ です。

変更前

note_md_—_fjord_note

変更後

ページ編集___FJORD_BOOT_CAMP(フィヨルドブートキャンプ)

@isshi-hasegawa isshi-hasegawa self-assigned this Apr 29, 2022
@isshi-hasegawa
Copy link
Contributor Author

@isshi-hasegawa isshi-hasegawa force-pushed the feature/replace-company-name-to-link branch from 2529260 to 42daf1c Compare May 2, 2022 06:54
@isshi-hasegawa isshi-hasegawa marked this pull request as ready for review May 2, 2022 10:59
@isshi-hasegawa
Copy link
Contributor Author

@Saki-htr
初レビュー依頼です!よろしくおねがいします〜

Copy link
Contributor

@Saki-htr Saki-htr left a comment

Choose a reason for hiding this comment

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

@isshi-hasegawa さん

初PRおめでとうございます ☺️
動作確認したところ、OKでした🙆‍♀️
「企業名のリンクが、その会社の個別ページのリンクになっているか」のテストも追加されていて素晴らしいと思います👏👏 assert has_link?の使い方をあまり知らなかったので勉強になりました!
私の方からはApproveさせていただきます〜


descriptionについて少し気になった箇所があったので、以下にコメントしました。好みによる部分と思いますので、ご変更していただかなくて大丈夫です〜。
今後のPR作成の際のご参考になれば幸いです🙏

descriptionについて

  1. すでにissueの方に記載はされているのですが、PullRequestのdescriptionにも、確認するURLを書いてくださると、レビュワーが動作確認しやすくて良さそうだと思いました。
  2. /admin/companiesには、タブが複数あるので、「企業」タブをクリックすることをテキストで書いていただくか、スクリーンショットを少し引いて撮っていただけると、より確認場所が分かりやすくなるかなと思いました。

image

@isshi-hasegawa
Copy link
Contributor Author

@Saki-htr
レビューありがとうございます〜
Descriptionについては、Sakiさんの仰るとおりにしたほうがよさそうです。
レビュワーの視点に立てていなかったので、ご指摘感謝です😊

@isshi-hasegawa
Copy link
Contributor Author

@komagata
メンバーのレビューが通りましたので、お手すきの際にレビューおねがいします〜

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 faa1e01 into main May 8, 2022
@komagata komagata deleted the feature/replace-company-name-to-link branch May 8, 2022 05:02
@github-actions github-actions bot mentioned this pull request May 8, 2022
16 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.

3 participants