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 repository for AppendComment #262

Conversation

gogog22510
Copy link
Contributor

👏 解決掉的 issue / Resolved Issues

📝 相關的 issue / Related Issues

⛏ 變更內容 / Details of Changes

  • 在repositoy.article裡串接bbs.DB裡的AppendNewLine

@codecov-commenter
Copy link

codecov-commenter commented Aug 3, 2021

Codecov Report

Merging #262 (98d1c4a) into development (bb21259) will decrease coverage by 0.13%.
The diff coverage is 0.00%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development     #262      +/-   ##
===============================================
- Coverage        45.49%   45.35%   -0.14%     
===============================================
  Files               29       29              
  Lines             1622     1627       +5     
===============================================
  Hits               738      738              
- Misses             782      787       +5     
  Partials           102      102              
Impacted Files Coverage Δ
internal/repository/article.go 0.00% <0.00%> (ø)

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 bb21259...98d1c4a. Read the comment docs.

@gogog22510 gogog22510 changed the title Implement repository for AppendComment partially Implement repository for AppendComment Aug 3, 2021
@PichuChen
Copy link
Member

看起來在等 go-bbs 的部分嗎?

@gogog22510
Copy link
Contributor Author

看起來在等 go-bbs 的部分嗎?

對,可能需要go-bbs增加一些method, 不知道go-bbs方面有沒有相關的issue了?

@PichuChen
Copy link
Member

Append的部分還沒有

@PichuChen
Copy link
Member

@gogog22510 有 Conflict 你那邊會處理嗎?

@gogog22510
Copy link
Contributor Author

我會處理conflict的部分

@PichuChen
Copy link
Member

@gogog22510 想問一下這邊狀況如何了?

@gogog22510 gogog22510 force-pushed the feature/232-update_usefulness_repo branch from 98d1c4a to a124919 Compare August 16, 2021 16:30
@gogog22510
Copy link
Contributor Author

@gogog22510 想問一下這邊狀況如何了?
解決conflict並且更新了go-bbs版本

@gogog22510 gogog22510 force-pushed the feature/232-update_usefulness_repo branch from a124919 to d48ce0b Compare August 25, 2021 00:44
@gogog22510
Copy link
Contributor Author

請問這個PR還需要做什麼嗎?

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

@PichuChen
Copy link
Member

請問這個PR還需要做什麼嗎?

看來是OK 看有沒有其他人幫忙 Review這樣

@y2468101216
Copy link
Collaborator

LGTM

@kyho4515
Copy link
Contributor

kyho4515 commented Aug 27, 2021

幾個小地方想問, PushRecord的部分要在usecase產生嗎? 如果這樣建議把return的type修一下

另外, 跟這個PR有點關係的是, 如果推文的話, 應該會有推文的text, 但我看delivery那邊似乎沒傳到usecase裏, 但repo的input有text, 這部分delivery那邊可能要改一下?還有如果有appendtype(上下箭頭)和text要寫進file裡的話, 那是不是要寫進appendtype+userID+text才是ptt推文後的樣子?

附上delivery的連結好討論一點
delivery

@PichuChen
Copy link
Member

幾個小地方想問, PushRecord的部分要在usecase產生嗎? 如果這樣建議把return的type修一下

另外, 跟這個PR有點關係的是, 如果推文的話, 應該會有推文的text, 但我看delivery那邊似乎沒傳到usecase裏, 但repo的input有text, 這部分delivery那邊可能要改一下?還有如果有appendtype(上下箭頭)和text要寫進file裡的話, 那是不是要寫進appendtype+userID+text才是ptt推文後的樣子?

附上delivery的連結好討論一點
delivery

聽起來要討論的部分應該是在usecase那邊討論。

另外我不是很清楚你說的和return type 修一下是建議修成怎樣?

@PichuChen PichuChen merged commit 033fc5d into Ptt-official-app:development Aug 27, 2021
@kyho4515
Copy link
Contributor

聽起來要討論的部分應該是在usecase那邊討論。

另外我不是很清楚你說的和return type 修一下是建議修成怎樣?

算是把return PushRecord, error 改成只有return error吧. 目前這樣也是ok.

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] 實作上下箭頭推文 - repository
5 participants