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

tag機能の追加 #169

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

tag機能の追加 #169

wants to merge 3 commits into from

Conversation

ryoha000
Copy link
Member

#135
タグの作成、付与、削除をついかしました
modelのテストが冗長な気がする

Copy link
Contributor

@nagatea nagatea left a comment

Choose a reason for hiding this comment

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

レビューしました
多少冗長になってもエラーチェックはしていくようにしよう
特にmodel.CreateItemは制約が少し厳しい(同じ名前は登録できない)のでテストが動かなくなる原因になるのでどこでテストが落ちるのかの切り分けにも使える

Comment on lines 44 to 47
item, _ := model.CreateItem(model.Item{Name: "testPostTags2ItemItem"})
tag1, _ := model.CreateTag("testPostTags2ItemTag1")
tag2, _ := model.CreateTag("testPostTags2ItemTag2")
tag3, _ := model.CreateTag("testPostTags2ItemTag3")
Copy link
Contributor

Choose a reason for hiding this comment

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

エラーチェックした方がいいですね
上でassert := assert.New(t)して下のやつ消せばOK

Comment on lines 92 to 93
item, _ := model.CreateItem(model.Item{Name: "testDeleteTagSuccessItem"})
tag, _ := model.CreateTag("testDeleteTagSuccessTag")
Copy link
Contributor

Choose a reason for hiding this comment

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

ここもエラーチェック

Comment on lines 78 to 79
item, _ := model.CreateItem(model.Item{Name: "testDeleteTagFailItem"})
tag, _ := model.CreateTag("testDeleteTagFailTag")
Copy link
Contributor

Choose a reason for hiding this comment

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

ここもエラーチェック

Copy link
Contributor

@nagatea nagatea left a comment

Choose a reason for hiding this comment

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

LGTM:+1:
クライアント側のUIについては今後考えていきましょ〜

@nagatea nagatea added the enhancement New feature or request label Oct 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants