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

Refactor predeployed contract wrapper classes #414

Merged
merged 9 commits into from
Mar 10, 2023
Merged

Refactor predeployed contract wrapper classes #414

merged 9 commits into from
Mar 10, 2023

Conversation

bigherc18
Copy link
Contributor

@bigherc18 bigherc18 commented Feb 28, 2023 •
edited
Loading

Usage related changes

No change affects the users

Development related changes

Predeployed contract wrapper classes now have a common parent class called PredeployedContractWrapper that should be inherited if we add other classes in the future.

Closes #325

Checklist:

  • Applied formatting - ./scripts/format.sh
  • No linter errors - ./scripts/lint.sh
  • Performed code self-review
  • Rebased to the last commit of the target branch (or merged it into my branch)
  • Documented the changes
  • Linked the issues which this PR resolves
  • Updated the tests
  • All tests are passing - ./scripts/test.sh

@bigherc18 bigherc18 requested a review from FabijanC March 4, 2023 18:30
@bigherc18 bigherc18 requested a review from FabijanC March 8, 2023 20:50
FabijanC
FabijanC suggested changes Mar 9, 2023
View reviewed changes
starknet_devnet/predeployed_contract_wrapper.py Outdated Show resolved Hide resolved
starknet_devnet/predeployed_contract_wrapper.py Outdated Show resolved Hide resolved
@FabijanC
Copy link
Collaborator

FabijanC commented Mar 9, 2023

Also, can you update your branch? It is out-of-date with the base branch

@bigherc18 bigherc18 requested a review from FabijanC March 10, 2023 09:51
FabijanC
FabijanC reviewed Mar 10, 2023
View reviewed changes
Copy link
Collaborator

@FabijanC FabijanC left a comment

Choose a reason for hiding this comment

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

Ok, looks nicer! Just left one comment.

starknet_devnet/fee_token.py Outdated Show resolved Hide resolved
@bigherc18 bigherc18 requested a review from FabijanC March 10, 2023 15:26
FabijanC
FabijanC approved these changes Mar 10, 2023
View reviewed changes
Copy link
Collaborator

@FabijanC FabijanC left a comment

Choose a reason for hiding this comment

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

If you can tell me that ./scripts/test.sh is passing, we can merge this. If we don't merge this today, we probably will by Tuesday.

Thanks for the cooperation and all the suggestions!

@bigherc18
Copy link
Contributor Author

@FabijanC Yes, tests are passing

@FabijanC FabijanC merged commit f946eb5 into 0xSpaceShard:master Mar 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@FabijanC FabijanC FabijanC approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Refactor predeployed contract wrapper classes
2 participants
@bigherc18 @FabijanC