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

Implement http for user information and enhance test case #159

Conversation

gogog22510
Copy link
Contributor

@gogog22510 gogog22510 commented Apr 2, 2021

👏 解決掉的 issue / Resolved Issues

📝 相關的 issue / Related Issues

⛏ 變更內容 / Details of Changes

  • 修改error message
  • 補齊testcode

@codecov-io
Copy link

codecov-io commented Apr 2, 2021

Codecov Report

Merging #159 (30d7a19) into development (c3d9f34) will decrease coverage by 1.33%.
The diff coverage is 5.12%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development     #159      +/-   ##
===============================================
- Coverage        39.93%   38.60%   -1.34%     
===============================================
  Files               22       22              
  Lines              969     1005      +36     
===============================================
+ Hits               387      388       +1     
- Misses             526      561      +35     
  Partials            56       56              
Impacted Files Coverage Δ
internal/usecase/token.go 0.00% <0.00%> (ø)
internal/usecase/usecase.go 100.00% <ø> (ø)
internal/usecase/user.go 5.34% <0.00%> (-0.40%) ⬇️
internal/delivery/http/route_users.go 53.21% <33.33%> (-1.00%) ⬇️
internal/delivery/http/route.go 62.16% <100.00%> (+0.34%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c3d9f34...30d7a19. Read the comment docs.

@gogog22510 gogog22510 force-pushed the feature/100-user_information_http branch 3 times, most recently from f98d4ba to 3d9d697 Compare April 2, 2021 04:40
internal/usecase/token.go Outdated Show resolved Hide resolved
Copy link
Member

@PichuChen PichuChen left a comment

Choose a reason for hiding this comment

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

請先按照 ISSUE 上面的實作,如果有其他要修改的請另外開 ISSUE 討論。

@gogog22510 gogog22510 force-pushed the feature/100-user_information_http branch 2 times, most recently from c128196 to 8086129 Compare April 4, 2021 01:33
@PichuChen
Copy link
Member

@gogog22510 好像有conflicts 要處理

@gogog22510 gogog22510 force-pushed the feature/100-user_information_http branch from 8086129 to e88fe88 Compare April 6, 2021 19:51
@gogog22510
Copy link
Contributor Author

請先按照 ISSUE 上面的實作,如果有其他要修改的請另外開 ISSUE 討論。

原本的功能已經有實作了,我照著ISSUE上所描述的,做了修改test case以及logging error的動作

@gogog22510 gogog22510 closed this Apr 6, 2021
@gogog22510 gogog22510 reopened this Apr 6, 2021
Copy link
Member

@PichuChen PichuChen left a comment

Choose a reason for hiding this comment

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

我現在看到他的 Files changed = 0 ,是我這邊有問題嗎?
截圖 2021-04-10 上午12 14 17

internal/delivery/http/route_users_test.go Outdated Show resolved Hide resolved
@gogog22510
Copy link
Contributor Author

gogog22510 commented Apr 9, 2021 via email

@PichuChen
Copy link
Member

但是在寫測試上面,應該是直接把map用key比較會比較確定發生了什麼事?

@gogog22510 gogog22510 force-pushed the feature/100-user_information_http branch from 9468f86 to 499f837 Compare April 9, 2021 18:35
@gogog22510
Copy link
Contributor Author

改成比對key了

@gogog22510 gogog22510 force-pushed the feature/100-user_information_http branch from 499f837 to eb085cc Compare April 13, 2021 13:59
Copy link
Contributor

@eternalriverco eternalriverco left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@PichuChen PichuChen left a comment

Choose a reason for hiding this comment

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

LGTM

@y2468101216
Copy link
Collaborator

LGTM

@PichuChen PichuChen merged commit 2170aae into Ptt-official-app:development Apr 20, 2021
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.

[主線] [PTT] 實作個人看板的使用者資訊的http套件
5 participants