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

Four new cost functions #306

Merged
merged 8 commits into from
Apr 26, 2022
Merged

Four new cost functions #306

merged 8 commits into from
Apr 26, 2022

Conversation

wycccccc
Copy link
Collaborator

完成 #298 第一階段
它們大致會長這樣
等待 #303 合併後解決 costFunctions conflict

@wycccccc wycccccc requested a review from chia7712 April 20, 2022 16:51
@wycccccc
Copy link
Collaborator Author

@chia7712 按照新的HasBrokerCost架構做了修改,現在他們彼此都很獨立。再麻煩學長review一下。

Copy link
Contributor

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

@wycccccc 這次的PR整齊目的又明確,讚讚

兩個建議請看一下

Copy link
Contributor

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

@wycccccc 有那個樣子了,不過犯了一個語法錯誤,請看一下

Copy link
Contributor

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

@wycccccc 還有一些想法,請參考

@chia7712 chia7712 requested a review from chinghongfang April 23, 2022 17:04
@wycccccc
Copy link
Collaborator Author

@chia7712 上述conversation已被解決,再麻煩學長review

Copy link
Contributor

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

@wycccccc 感謝更新,大部分看起來不錯。有一個比較大的設計問題在底下,看看大家的想法

chia7712
chia7712 previously approved these changes Apr 26, 2022
Copy link
Contributor

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

@wycccccc 這次的主題明確、程式碼也很乾淨俐落!

補完測試後就可以合併了,謝謝

@wycccccc wycccccc merged commit ba6d2de into opensource4you:main Apr 26, 2022
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