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

refactor(verification): move token info [part 2b/9] #858

Merged
merged 1 commit into from
Nov 16, 2023

Conversation

glevco
Copy link
Contributor

@glevco glevco commented Nov 9, 2023

Depends on #831

Motivation

During the verification refactors, some methods were moved from Transaction to TransactionVerifier. Now that the refactors are complete (with PRs still to be merged), it's clear that those methods should be indeed in Transaction, so they were moved back.

Acceptance Criteria

  • Move update_token_info_from_outputs() from TransactionVerifier to Transaction, changing it to private.
  • Move get_complete_token_info() from TransactionVerifier to Transaction.
  • Change Transaction.get_token_info_from_inputs() to private.

Checklist

  • If you are requesting a merge into master, confirm this code is production-ready and can be included in future releases as soon as it gets merged

Copy link

codecov bot commented Nov 9, 2023

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (351e4d3) 85.28% compared to head (5693c13) 85.27%.

Files Patch % Lines
hathor/transaction/transaction.py 90.47% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #858      +/-   ##
==========================================
- Coverage   85.28%   85.27%   -0.02%     
==========================================
  Files         281      281              
  Lines       22363    22364       +1     
  Branches     3388     3388              
==========================================
- Hits        19073    19071       -2     
  Misses       2610     2610              
- Partials      680      683       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@glevco glevco force-pushed the refactor/verification-inheritance/2-verify-sum branch from ba246bc to c69073d Compare November 9, 2023 19:22
@glevco glevco force-pushed the refactor/move-token-info branch from 8cff879 to a7635d4 Compare November 9, 2023 19:22
@glevco glevco force-pushed the refactor/verification-inheritance/2-verify-sum branch from c69073d to 2e709e9 Compare November 9, 2023 19:27
@glevco glevco force-pushed the refactor/move-token-info branch from a7635d4 to 1bef803 Compare November 9, 2023 19:28
@glevco glevco marked this pull request as ready for review November 9, 2023 19:38
msbrogli
msbrogli previously approved these changes Nov 9, 2023
@glevco glevco force-pushed the refactor/verification-inheritance/2-verify-sum branch 2 times, most recently from dc55893 to 073d6f6 Compare November 10, 2023 16:31
@glevco glevco force-pushed the refactor/move-token-info branch from 1bef803 to 9857b55 Compare November 10, 2023 16:56
@glevco glevco changed the title refactor: move token info refactor(verification): move token info [part 2b/9] Nov 10, 2023
@glevco glevco force-pushed the refactor/verification-inheritance/2-verify-sum branch from 073d6f6 to c084103 Compare November 16, 2023 17:02
@glevco glevco force-pushed the refactor/move-token-info branch from 9857b55 to 50171eb Compare November 16, 2023 17:03
Base automatically changed from refactor/verification-inheritance/2-verify-sum to master November 16, 2023 20:01
@glevco glevco dismissed msbrogli’s stale review November 16, 2023 20:01

The base branch was changed.

@glevco glevco force-pushed the refactor/move-token-info branch from 50171eb to 5693c13 Compare November 16, 2023 20:03
@glevco glevco merged commit 29a622f into master Nov 16, 2023
9 checks passed
@glevco glevco deleted the refactor/move-token-info branch November 16, 2023 21:21
@jansegre jansegre mentioned this pull request Nov 24, 2023
2 tasks
This was referenced Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants