Skip to content
This repository has been archived by the owner on Oct 31, 2023. It is now read-only.

Migrate Payments to TaskPayment #4240

Merged
merged 16 commits into from
Jun 5, 2019
Merged

Migrate Payments to TaskPayment #4240

merged 16 commits into from
Jun 5, 2019

Conversation

jiivan
Copy link
Contributor

@jiivan jiivan commented May 27, 2019

fixes: #4197
additionally fixes: #2457

@jiivan jiivan requested review from Krigpl and kmazurek May 27, 2019 14:27
@jiivan jiivan marked this pull request as ready for review May 29, 2019 10:42
if self.details is None:
self.details = PaymentDetails()

def __repr__(self) -> str:
Copy link
Contributor

Choose a reason for hiding this comment

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

__repr__ is missing on both of the newly introduced classes, is that intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we'll add them, when they're needed?

Copy link
Contributor

@Krigpl Krigpl left a comment

Choose a reason for hiding this comment

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

Please, no unnecessary golem-messages dependencies in the ethereum module.

Copy link
Contributor

@Krigpl Krigpl left a comment

Choose a reason for hiding this comment

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

Thanks, that looks muuuch better. Mostly lgtm but have a couple of questions inline.

jiivan added 2 commits June 5, 2019 11:03
It fails in lintdiff.sh (confusion with ethereum module)
@codecov
Copy link

codecov bot commented Jun 5, 2019

Codecov Report

Merging #4240 into develop will increase coverage by 0.08%.
The diff coverage is 82.6%.

@@             Coverage Diff             @@
##           develop    #4240      +/-   ##
===========================================
+ Coverage    88.56%   88.64%   +0.08%     
===========================================
  Files          223      224       +1     
  Lines        19694    19643      -51     
===========================================
- Hits         17442    17413      -29     
+ Misses        2252     2230      -22

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate Payments to Transaction Remove dead code from PaymentsKeeper & PaymentsDatabase
3 participants