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

Doc, naming, ordering, copyrights #157

Merged
merged 5 commits into from
Dec 17, 2022
Merged

Conversation

neodaoist
Copy link
Contributor

@neodaoist neodaoist commented Dec 16, 2022

  • Clarify doc
  • Update member visibility
  • Update member ordering
  • Tweak internal naming
  • Standardize copyrights

Copy link
Member

@0xAlcibiades 0xAlcibiades left a comment

Choose a reason for hiding this comment

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

A few change requests, and needs to pass lint, but otherwise looking nice.

src/interfaces/IOptionSettlementEngine.sol Outdated Show resolved Hide resolved
src/interfaces/IOptionSettlementEngine.sol Outdated Show resolved Hide resolved
src/OptionSettlementEngine.sol Outdated Show resolved Hide resolved
src/OptionSettlementEngine.sol Outdated Show resolved Hide resolved
src/OptionSettlementEngine.sol Outdated Show resolved Hide resolved
src/OptionSettlementEngine.sol Outdated Show resolved Hide resolved
src/OptionSettlementEngine.sol Outdated Show resolved Hide resolved
src/OptionSettlementEngine.sol Outdated Show resolved Hide resolved
@neodaoist
Copy link
Contributor Author

A few change requests, and needs to pass lint, but otherwise looking nice.

Not sure what is failing lint, def ran it, and nothing happening locally when I run. Will investigate what happens in the build on this next push.

@codecov-commenter
Copy link

codecov-commenter commented Dec 16, 2022

Codecov Report

Merging #157 (fc35a41) into audit-fixes (18b2a9e) will not change coverage.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##           audit-fixes     #157   +/-   ##
============================================
  Coverage        90.00%   90.00%           
============================================
  Files                2        2           
  Lines              370      370           
  Branches            55       55           
============================================
  Hits               333      333           
  Misses              29       29           
  Partials             8        8           
Impacted Files Coverage Δ
src/TokenURIGenerator.sol 75.78% <ø> (ø)
src/OptionSettlementEngine.sol 97.52% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@0xAlcibiades 0xAlcibiades merged commit 8686afe into audit-fixes Dec 17, 2022
@0xAlcibiades 0xAlcibiades deleted the refactor/naming-things branch December 17, 2022 00:17
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.

3 participants