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

Adds box and parsing modules #16

Merged
merged 3 commits into from
Jan 12, 2023
Merged

Conversation

AlgoStephenAkiki
Copy link
Contributor

No description provided.

@AlgoStephenAkiki AlgoStephenAkiki force-pushed the 1328-remove-logic-dependency branch from 3943708 to 4c2b1ee Compare January 6, 2023 19:52
@codecov-commenter
Copy link

codecov-commenter commented Jan 6, 2023

Codecov Report

Merging #16 (4c2b1ee) into main (55c07c8) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main      #16   +/-   ##
=======================================
  Coverage   73.66%   73.66%           
=======================================
  Files           3        3           
  Lines         972      972           
=======================================
  Hits          716      716           
  Misses        182      182           
  Partials       74       74           
Impacted Files Coverage Δ
abi/json.go 57.89% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@AlgoStephenAkiki AlgoStephenAkiki self-assigned this Jan 6, 2023
@AlgoStephenAkiki AlgoStephenAkiki marked this pull request as ready for review January 6, 2023 21:33
@winder winder requested a review from jasonpaulos January 9, 2023 14:48
@michaeldiamant
Copy link
Contributor

Copy link
Contributor

@michaeldiamant michaeldiamant left a comment

Choose a reason for hiding this comment

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

@AlgoStephenAkiki Matches my understanding of what we intended to move. As a sanity check, I suggest holding for 1 more approval prior to merge to minimize need for issuing multiple releases.

Copy link
Contributor

@jasonpaulos jasonpaulos left a comment

Choose a reason for hiding this comment

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

Looks pretty good to me, but I think I'd prefer if the address functions were moved into their own package instead of being in the existing abi package.

I opened a child PR that does this, could you take a look? #17

* Create address package

* Update function docs
@winder winder requested a review from jasonpaulos January 12, 2023 18:29
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.

5 participants