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

プラクティス個別ページのDocsのタブにDocの数を表示 #3406

Merged
merged 4 commits into from
Oct 26, 2021

Conversation

Ichiyo14
Copy link
Contributor

issue: #3392
プラクティス個別ページのDocsのタブにDocの数を表示した。

変更前イメージ

スクリーンショット 2021-10-17 10 32 47

変更後イメージ

スクリーンショット 2021-10-17 10 33 11

@Ichiyo14 Ichiyo14 marked this pull request as draft October 17, 2021 01:40
@Ichiyo14 Ichiyo14 marked this pull request as ready for review October 17, 2021 02:27
@Ichiyo14
Copy link
Contributor Author

@konaga-k
お時間がある時にレビューをよろしくお願いします🙏

@Ichiyo14 Ichiyo14 requested a review from konaga-k October 17, 2021 03:55
Copy link
Contributor

@konaga-k konaga-k left a comment

Choose a reason for hiding this comment

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

@Ichiyo14
OKです
test/fixtures/pages.yml の件は駒形さんの回答次第で必要なら対応お願いします
LGTM


PRのassigneeに自身を設定しておいてください〜
image

Comment on lines 46 to 52

page7:
title: プラクティスに紐付いたDocs
body: プラクティスに紐付いている
user: komagata
practice: practice1
published_at: "2021-10-01 00:00:00"
Copy link
Contributor

Choose a reason for hiding this comment

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

基本的に db/fixturestest/fixtures の同名ファイルの中身は一致させている気がします
なので test/fixtures/pages.yml にも page7 を追加して欲しいと思ったんですが、 pages.yml の場合は page6 の内容が違っちゃってますね…
https://github.com/fjordllc/bootcamp/blob/feature/show-the-number-of-docs-in-the-docs-tab/test/fixtures/pages.yml

対応方針が分からないので駒形さんに聞いてみる
@komagata
db/fixturestest/fixtures の同名ファイルの中身ってできるだけ一致させたほうがいいんでしょうか?
元から中身が一致しているものもあれば今回の pages.ymlpage6 のように不一致なものもあります
page6 のような既存のデータを今から一致させる必要まではないと思うんですが、これから追加するデータについては db/fixturestest/fixtures の両方に追加したほうがいいんでしょうか?

Copy link
Member

Choose a reason for hiding this comment

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

@konaga-k 一致させる必要はないです。

@Ichiyo14 Ichiyo14 self-assigned this Oct 17, 2021
@Ichiyo14
Copy link
Contributor Author

@konaga-k
レビューありがとうございます!

@komagata
上記の件のご確認と合わせてレビューをお願いします🙏

@machida machida requested a review from komagata October 19, 2021 06:14
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.

conflictの解消お願い致します〜

@Ichiyo14
Copy link
Contributor Author

@komagata
conflictの解消をしました! ご確認お願いします。

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 2807e61 into main Oct 26, 2021
@komagata komagata deleted the feature/show-the-number-of-docs-in-the-docs-tab branch October 26, 2021 05:38
@github-actions github-actions bot mentioned this pull request Oct 26, 2021
11 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