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

順位表の時間表示を一部修正  #182

Merged
merged 6 commits into from
Apr 25, 2021

Conversation

no-yan
Copy link
Collaborator

@no-yan no-yan commented Apr 20, 2021

close #170
コンテストページ得点欄の時間表示のフォーマットを変えました。
コミットメッセージで間違えてますが、フォーマットは分分:秒秒ですね。中身は大丈夫です。

周辺の型定義を追加、コンポーネントを切り出しました。
reactはコンポーネントをなるべく小さく分けて、レイアウト(ビュー)部分とロジック部分を独立させることがベストプラクティスのようです。コンポーネントを細かく分けることはパフォーマンス戦略としても優れていますし、独立性を高めることはメンテの容易さにも繋がります。

@rdrgn PRのリファクタリング部分で#133の作業とコンフリクトしている部分があれば、ご自身のファイルの方を優先してくださって大丈夫です!コンフリクト解決は面倒だと思うので……

@no-yan no-yan requested a review from ShopOne as a code owner April 20, 2021 20:58
@reminjp
Copy link
Collaborator

reminjp commented Apr 21, 2021

PRのリファクタリング部分で#133の作業とコンフリクトしている部分があれば、ご自身のファイルの方を優先してくださって大丈夫です!コンフリクト解決は面倒だと思うので……

お気遣いありがとうございます。問題の #133 は重すぎて半ばで放置しておりましたので、無視して大丈夫です。むしろリファクタリングを進めていただけてありがたいです。

Copy link
Owner

@ShopOne ShopOne left a comment

Choose a reason for hiding this comment

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

リファクタリングも含めて有難うございます!
いくつか指摘事項と補足コメントをさせていただいたので、ご確認のほど宜しくお願いします。

frontend/src/components/RankingTable.tsx Outdated Show resolved Hide resolved
frontend/src/components/RankingTable.tsx Outdated Show resolved Hide resolved
Copy link
Owner

@ShopOne ShopOne left a comment

Choose a reason for hiding this comment

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

再度プルリクエストを出して頂きありがとうございます!
細かい所で申し訳ないのですが、再度指摘事項があるのでそちらの修正をお願いしたいです。
これが終われば merge 出来ると思います。

frontend/src/components/RankingTable.tsx Outdated Show resolved Hide resolved
frontend/src/components/RankingTable.tsx Outdated Show resolved Hide resolved
frontend/src/components/RankingTable.tsx Outdated Show resolved Hide resolved
@no-yan
Copy link
Collaborator Author

no-yan commented Apr 22, 2021

丁寧なレビューありがとうございます。二度目の指摘は勇気がいりますが、受ける側としては率直に言ってもらえることでとてもやりやすく感じます。
(こちらからはreviewerを変更することができないので、 @ShopOne さんの負担が(雑務的な方向性で)増えそうなのを少し気にしています。)

PointAndAcTime関数はprobListでしか参照されないコンポーネントなので削除しました。

@ShopOne
Copy link
Owner

ShopOne commented Apr 23, 2021

こちらも内心ビクビクしながら Review を書いているので、そう行って頂けると精神的にとても助かります。ありがとうございます。
僕の作業量に関しては気にせず、どしどしプルリクエストを出してもらってOKです! (忙しい時は反応遅れちゃうかもですが)
commit の内容については大丈夫そうです! 再レビュー状態になっていないので、一応まだ merge は待機中です。
ついでのお願いですが、 conflict しているのを直して頂けると嬉しいです。(見た感じ probElement 消すだけっぽいですが)

Copy link
Owner

@ShopOne ShopOne left a comment

Choose a reason for hiding this comment

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

大丈夫そうです、ありがとうございます!

@ShopOne ShopOne merged commit 344598a into ShopOne:master Apr 25, 2021
@no-yan no-yan deleted the display_time_as_MMSS branch August 16, 2021 15:00
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.

feat: 順位表のペナルティ入りの最終AC時刻を「分:秒」で表示する
3 participants