Skip to content

refactor(scripts): modularize script files #811

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

Merged
merged 1 commit into from
Nov 7, 2023
Merged

Conversation

glevco
Copy link
Contributor

@glevco glevco commented Oct 17, 2023

Motivation

Refactor code so the single scripts.py file is broken into multiple modules, improving organization and maintainability. No behavior is changed by this PR, code is only moved between files. The only active changes are in imports to fix circular dependencies after the refactor.

Acceptance Criteria

  1. Move HathorScript to its own file
  2. Move BaseScript to its own file
  3. Move P2PKH to its own file
  4. Move MultiSig to its own file
  5. Move NanoContractMatchValues to its own file
  6. Move Opcode and functions to their own file
  7. Remaining functions from the scripts file moved to either construct or execute modules
  8. Fix imports and linter issues

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

@glevco glevco self-assigned this Oct 17, 2023
@glevco glevco changed the title Refactor/scripts refactor(scripts): modularize script files Oct 17, 2023
@glevco glevco force-pushed the refactor/scripts branch 3 times, most recently from efeac28 to 74afb2a Compare October 17, 2023 19:34
@codecov
Copy link

codecov bot commented Oct 17, 2023

Codecov Report

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

Comparison is base (3c8698e) 85.08% compared to head (7c774b1) 85.07%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #811      +/-   ##
==========================================
- Coverage   85.08%   85.07%   -0.02%     
==========================================
  Files         276      284       +8     
  Lines       22514    22576      +62     
  Branches     3430     3430              
==========================================
+ Hits        19157    19207      +50     
- Misses       2674     2683       +9     
- Partials      683      686       +3     
Files Coverage Δ
hathor/transaction/scripts/__init__.py 100.00% <100.00%> (ø)
hathor/transaction/scripts/base_script.py 100.00% <100.00%> (ø)
hathor/transaction/scripts/hathor_script.py 100.00% <100.00%> (ø)
hathor/transaction/scripts/multi_sig.py 97.67% <97.67%> (ø)
hathor/transaction/scripts/p2pkh.py 96.72% <96.72%> (ø)
hathor/transaction/scripts/execute.py 96.74% <96.74%> (ø)
hathor/transaction/scripts/opcode.py 97.02% <97.02%> (ø)
.../transaction/scripts/nano_contract_match_values.py 87.87% <87.87%> (ø)
hathor/transaction/scripts/construct.py 81.98% <81.98%> (ø)

... and 5 files with indirect coverage changes

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

@glevco glevco marked this pull request as ready for review October 18, 2023 02:19
@glevco glevco force-pushed the refactor/scripts branch 3 times, most recently from 0819fb0 to f21aa10 Compare October 25, 2023 17:24
@glevco glevco force-pushed the refactor/scripts branch 3 times, most recently from ac68a8d to 97bbcc4 Compare October 27, 2023 20:02
msbrogli
msbrogli previously approved these changes Oct 27, 2023
@glevco glevco force-pushed the refactor/scripts branch 2 times, most recently from ec5adde to e5a19b7 Compare November 5, 2023 16:42
@glevco glevco merged commit 769d255 into master Nov 7, 2023
@glevco glevco deleted the refactor/scripts branch November 7, 2023 01:56
@jansegre jansegre mentioned this pull request Nov 13, 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