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

未ログイン状態のトップ画面にGitHubのリンクを追加 #8161

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hagiya0121
Copy link
Contributor

@hagiya0121 hagiya0121 commented Oct 28, 2024

Issue

ログイン前にGitHubへのリンクが欲しい #8159

概要

未ログイン状態のトップ画面にリポジトリリンクを追加しました。

変更確認方法

  1. feature/add-github-linkをローカルに取り込む
  2. 'foreman start -f Procfile.dev 'でローカルサーバーを立ち上げ
  3. 未ログイン状態でトップ画面にアクセスしてフッター部分にリンクが追加さ
    れていることを確認
    http://localhost:3000/

Screenshot

変更前

image

変更後

227f7ed549bd662a6b2bf73312798d45a3bbe16f0c4d42506de71feb56b2d4e9

@hagiya0121 hagiya0121 self-assigned this Oct 28, 2024
@hagiya0121
Copy link
Contributor Author

@kurumadaisuke
お疲れ様です。
こちらのPRのレビューをお願いしたいです🙏
ご都合が悪いときは遠慮なくおっしゃってください🙇

@hagiya0121 hagiya0121 marked this pull request as ready for review October 29, 2024 09:38
@kurumadaisuke
Copy link
Contributor

@hagiya0121
すいません、ちょっと家の都合でバタバタしていまして別の方でも大丈夫でしょうか?🙇‍♂️

@hagiya0121
Copy link
Contributor Author

@kurumadaisuke
全然大丈夫です!

@hagiya0121
Copy link
Contributor Author

@mousu-a
お疲れ様です。
こちらのPRのレビューをお願いしたいです🙏
ご都合が悪いときは遠慮なくおっしゃってください🙇

@hagiya0121 hagiya0121 requested review from mousu-a and removed request for kurumadaisuke October 30, 2024 08:41
@mousu-a
Copy link
Contributor

mousu-a commented Oct 30, 2024

@hagiya0121
お疲れ様です!
レビュー了解です〜🙆
今週にはお返しできると思います!それでよろしければ引き受けさせていただきます〜

@hagiya0121
Copy link
Contributor Author

@mousu-a
いつでも大丈夫です!よろしくお願いします🙏

Copy link
Contributor

@mousu-a mousu-a left a comment

Choose a reason for hiding this comment

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

@hagiya0121
レビューお待たせしました🙇‍♂️

コード良いと思います!
PR概要の「変更確認方法」についてですが、こんなふうにするとより親切かもです〜🙆
(細かいところですがせっかくのgood first issueなので!)
image

git fetch origin pull/8161/head:feature/add-github-link
git checkout feature/add-github-link


他にも一点コメントさせていただきましたが修正はありませんのでこちらからはApproveとさせていただきます〜!

@@ -14,6 +14,9 @@ footer.not-logged-in-footer
li.not-logged-in-footer__nav-item
= link_to coc_path, class: 'not-logged-in-footer__nav-item-link' do
| アンチハラスメントポリシー
li.not-logged-in-footer__nav-item
= link_to 'https://github.com/fjordllc/bootcamp', class: 'not-logged-in-footer__nav-item-link' do
| ソースコード
Copy link
Contributor

@mousu-a mousu-a Nov 1, 2024

Choose a reason for hiding this comment

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

コミット 74c4962 についてですが、これくらいの変更点であればrebaseして1つのコミットにまとめてしまって良いと思います!
理由としましては、レビュアーは知らなくて良いコミット(履歴)だと思うからです👀
(今から修正する必要はないです!今後の参考までに)

Copy link
Member

Choose a reason for hiding this comment

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

@hagiya0121 (CC: @mousu-a )

こちらできれば修正していただきたいです〜

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@komagata
修正しました🙏

@hagiya0121
Copy link
Contributor Author

@mousu-a
レビューありがとうございます!指摘いただいたこと、2点とも知らなかったことなのですごい参考になりました🙏

@hagiya0121
Copy link
Contributor Author

@komagata
メンバーのレビューが完了したのでレビューお願いします🙏

@machida
Copy link
Member

machida commented Nov 6, 2024

Github → GitHub ですー

@hagiya0121 hagiya0121 changed the title 未ログイン状態のトップ画面にGithubのリンクを追加 未ログイン状態のトップ画面にGitHubのリンクを追加 Nov 6, 2024
@hagiya0121
Copy link
Contributor Author

@machida
ありがとうございます🙏修正しておきました。

@@ -14,6 +14,9 @@ footer.not-logged-in-footer
li.not-logged-in-footer__nav-item
= link_to coc_path, class: 'not-logged-in-footer__nav-item-link' do
| アンチハラスメントポリシー
li.not-logged-in-footer__nav-item
= link_to 'https://github.com/fjordllc/bootcamp', class: 'not-logged-in-footer__nav-item-link' do
| ソースコード
Copy link
Member

Choose a reason for hiding this comment

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

@hagiya0121 (CC: @mousu-a )

こちらできれば修正していただきたいです〜

@mousu-a
Copy link
Contributor

mousu-a commented Nov 15, 2024

@komagata
すみません、コメントの意図がわからなかったのですが、

#8077 (comment)

こちらのissueと同じようにtarget: '_blank', rel: 'noopener'をつけてほしい、ということでしょうか?

@hagiya0121
Copy link
Contributor Author

@mousu-a
質問タイムでお聞きしたところmousuさんがレビューで指摘したようにコミットをまとめてほしいとのことでした🙇

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.

5 participants