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

Controller refactoring #1033

Merged
merged 39 commits into from
Jun 20, 2023
Merged

Controller refactoring #1033

merged 39 commits into from
Jun 20, 2023

Conversation

marcocastignoli
Copy link
Member

@marcocastignoli marcocastignoli commented May 15, 2023

View in Huly HI-551

@codecov-commenter
Copy link

codecov-commenter commented Jun 1, 2023

Codecov Report

Patch coverage: 78.11% and project coverage change: -2.98 ⚠️

Comparison is base (0a21ecc) 77.65% compared to head (8acd9a5) 74.68%.

Additional details and impacted files
@@             Coverage Diff             @@
##           staging    #1033      +/-   ##
===========================================
- Coverage    77.65%   74.68%   -2.98%     
===========================================
  Files           31       66      +35     
  Lines         1450     2354     +904     
  Branches       266      431     +165     
===========================================
+ Hits          1126     1758     +632     
- Misses         188      370     +182     
- Partials       136      226      +90     
Flag Coverage Δ
lib-sourcify 66.70% <ø> (?)
server 78.94% <78.11%> (+1.28%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ontrollers/testartifacts/testartifacts.handlers.ts 10.00% <10.00%> (ø)
src/server/middlewares/GenericErrorHandler.ts 66.66% <25.00%> (-6.07%) ⬇️
...cation/create2/session/create2.session.handlers.ts 56.66% <56.66%> (ø)
...olc-json/stateless/solc-json.stateless.handlers.ts 59.37% <59.37%> (ø)
...er/controllers/verification/verification.common.ts 81.20% <63.26%> (ø)
...on/create2/stateless/create2.stateless.handlers.ts 68.18% <68.18%> (ø)
src/sourcify-chains.ts 84.21% <70.00%> (-0.74%) ⬇️
src/server/server.ts 82.29% <73.33%> (-5.64%) ⬇️
...rification/session-state/session-state.handlers.ts 75.47% <75.47%> (ø)
src/server/common.ts 75.75% <75.75%> (ø)
... and 30 more

... and 9 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@marcocastignoli marcocastignoli marked this pull request as ready for review June 5, 2023 07:40
* services as instantiated global variable
* first new filesystem proposal
* add /verify and / routes, not working together
* add /verify/solc-json, need to complete
* restore express-fileupload
* fix redirect not working for /
marcocastignoli and others added 4 commits June 12, 2023 14:54
* Change serDes to format validation

* Remove commented section
Add prod. staging. local servers to openAPI

Add MIT license
@kuzdogan
Copy link
Member

Rebased and force pushed to solve the npx lerna issue

src/server/controllers/repository/repository.handlers.ts Outdated Show resolved Hide resolved
src/server/controllers/repository/repository.routes.ts Outdated Show resolved Hide resolved
.gitignore Show resolved Hide resolved
test/helpers/etherscanInstanceContracts.json Show resolved Hide resolved
openapi.yaml Show resolved Hide resolved
@kuzdogan
Copy link
Member

kuzdogan commented Jun 12, 2023

Looks fine overall. Left some comments, please check them out.

Two main things missing:
1. The repo.sourcify.dev part of the API is non-existent: https://docs.sourcify.dev/docs/api/#repository as well as the other endpoints: https://docs.sourcify.dev/docs/api/#other
How can we add the repo.sourcify.dev into the SwaggerUI? Can we create another view with a different server? Or when the user selects repo.sourcify.dev can we render a different UI?
For this in general, maybe double check against the existing https://docs.sourcify.dev/docs/api/

  1. For others can you double-check example requests and responses? I see for example /verify and /verify/create2's example request is missing as well as the responses. For verification responses you can maybe create a shared errors .yaml as most of the errors are shared.
    I see you directly copied the errors at https://docs.sourcify.dev/docs/api/server/verify/#responses but I am not sure if those are up-to-date.

Copy link
Member

@kuzdogan kuzdogan left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you!

@kuzdogan kuzdogan merged commit b22dfab into staging Jun 20, 2023
@kuzdogan kuzdogan deleted the controller-refactoring branch December 19, 2023 18:22
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.

4 participants