-
Notifications
You must be signed in to change notification settings - Fork 13
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
順位表の時間表示を一部修正 #182
順位表の時間表示を一部修正 #182
Conversation
お気遣いありがとうございます。問題の #133 は重すぎて半ばで放置しておりましたので、無視して大丈夫です。むしろリファクタリングを進めていただけてありがたいです。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
リファクタリングも含めて有難うございます!
いくつか指摘事項と補足コメントをさせていただいたので、ご確認のほど宜しくお願いします。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
再度プルリクエストを出して頂きありがとうございます!
細かい所で申し訳ないのですが、再度指摘事項があるのでそちらの修正をお願いしたいです。
これが終われば merge 出来ると思います。
丁寧なレビューありがとうございます。二度目の指摘は勇気がいりますが、受ける側としては率直に言ってもらえることでとてもやりやすく感じます。 PointAndAcTime関数はprobListでしか参照されないコンポーネントなので削除しました。 |
こちらも内心ビクビクしながら Review を書いているので、そう行って頂けると精神的にとても助かります。ありがとうございます。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
大丈夫そうです、ありがとうございます!
close #170
コンテストページ得点欄の時間表示のフォーマットを変えました。
コミットメッセージで間違えてますが、フォーマットは分分:秒秒ですね。中身は大丈夫です。
周辺の型定義を追加、コンポーネントを切り出しました。
reactはコンポーネントをなるべく小さく分けて、レイアウト(ビュー)部分とロジック部分を独立させることがベストプラクティスのようです。コンポーネントを細かく分けることはパフォーマンス戦略としても優れていますし、独立性を高めることはメンテの容易さにも繋がります。
@rdrgn PRのリファクタリング部分で#133の作業とコンフリクトしている部分があれば、ご自身のファイルの方を優先してくださって大丈夫です!コンフリクト解決は面倒だと思うので……