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.

Sign up for GitHub

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

Jump to bottom

refactor: remove legacy REST APIs #807

Merged
merged 22 commits into from
Dec 8, 2022
Merged

refactor: remove legacy REST APIs #807

merged 22 commits into from
Dec 8, 2022

Conversation

tkxkd0159
Copy link
Member

@tkxkd0159 tkxkd0159 commented Nov 25, 2022 •
edited
Loading

Description

closes: #718
This PR also remove x/ibc, x/wasm legacy REST API which are not scoped in cosmos-sdk's PR. Because they don't belong to the official spec of cosmos-sdk. And we moved types/rest package to testutil/rest

Motivation and context

How has this been tested?

Screenshots (if appropriate):

Checklist:

  • I followed the contributing guidelines and code of conduct.
  • I have added a relevant changelog to CHANGELOG.md
  • I have added tests to cover my changes.
  • I have updated the documentation accordingly.
  • I have updated API documentation client/docs/swagger-ui/swagger.yaml

@tkxkd0159 tkxkd0159 changed the title remove legacy REST API from swagger remove legacy REST API Nov 25, 2022
@tkxkd0159 tkxkd0159 changed the title remove legacy REST API refactor: remove legacy REST API Nov 25, 2022
@codecov
Copy link

codecov bot commented Nov 25, 2022 •
edited
Loading

Codecov Report

Merging #807 (a50a5c4) into main (ffb28cd) will increase coverage by 1.34%.
The diff coverage is 95.95%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #807      +/-   ##
==========================================
+ Coverage   61.75%   63.09%   +1.34%     
==========================================
  Files         883      869      -14     
  Lines      100381   100419      +38     
==========================================
+ Hits        61988    63358    +1370     
+ Misses      34775    33533    -1242     
+ Partials     3618     3528      -90
Impacted Files Coverage Δ
client/rpc/block.go 0.00% <ø> (-46.56%) ⬇️
client/rpc/status.go 64.28% <ø> (+16.74%) ⬆️
client/rpc/validators.go 0.00% <0.00%> (-1.54%) ⬇️
client/tx/tx.go 39.84% <ø> (+6.61%) ⬆️
simapp/app.go 85.31% <0.00%> (+0.47%) ⬆️
testutil/network/network.go 81.66% <ø> (-0.06%) ⬇️
types/module/module.go 67.36% <ø> (-0.54%) ⬇️
types/query/pagination.go 80.00% <ø> (ø)
x/auth/client/cli/query.go 1.92% <0.00%> (ø)
x/gov/client/testutil/grpc.go 100.00% <ø> (ø)
... and 23 more

@tkxkd0159 tkxkd0159 marked this pull request as ready for review November 25, 2022 19:14
@tkxkd0159 tkxkd0159 self-assigned this Nov 30, 2022
@tkxkd0159 tkxkd0159 force-pushed the remove-legacy-rest branch 2 times, most recently from 0a35931 to 4c73cdb Compare December 1, 2022 10:50
ulbqb
ulbqb requested changes Dec 2, 2022
View reviewed changes
Copy link
Member

@ulbqb ulbqb left a comment

Choose a reason for hiding this comment

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

It seem to have many unnecessary comment fix. It's in other places than I pointed out.

x/ibc/core/02-client/types/msgs.go Outdated Show resolved Hide resolved
x/ibc/core/03-connection/keeper/handshake.go Outdated Show resolved Hide resolved
x/ibc/light-clients/99-ostracon/types/upgrade.go Outdated Show resolved Hide resolved
x/ibc/applications/transfer/types/msgs.go Outdated Show resolved Hide resolved
x/ibc/applications/transfer/types/trace.go Outdated Show resolved Hide resolved
x/ibc/core/03-connection/types/msgs.go Outdated Show resolved Hide resolved
x/ibc/core/04-channel/keeper/handshake.go Outdated Show resolved Hide resolved
x/ibc/core/24-host/doc.go Outdated Show resolved Hide resolved
x/ibc/core/keeper/msg_server_test.go Outdated Show resolved Hide resolved
x/ibc/light-clients/99-ostracon/types/proposal_handle.go Outdated Show resolved Hide resolved
@tkxkd0159 tkxkd0159 changed the title refactor: remove legacy REST API refactor: remove legacy REST APIs Dec 2, 2022
@tkxkd0159 tkxkd0159 requested a review from ulbqb December 2, 2022 04:43
ulbqb
ulbqb approved these changes Dec 5, 2022
View reviewed changes
Copy link
Member

@ulbqb ulbqb left a comment

Choose a reason for hiding this comment

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

Regarding IBC, this change seems to be same as this.
LGTM

dudong2
dudong2 reviewed Dec 6, 2022
View reviewed changes
tests/mocks/types_handler.go Show resolved Hide resolved
tests/mocks/account_retriever.go Show resolved Hide resolved
x/bank/client/testutil/grpc.go Show resolved Hide resolved
zemyblue
zemyblue reviewed Dec 6, 2022
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
zemyblue
zemyblue reviewed Dec 6, 2022
View reviewed changes
simapp/app.go Show resolved Hide resolved
@tkxkd0159 tkxkd0159 merged commit ada8700 into Finschia:main Dec 8, 2022
@tkxkd0159 tkxkd0159 mentioned this pull request Dec 8, 2022
Merged
3 tasks
@zemyblue zemyblue mentioned this pull request Feb 12, 2023
Closed
8 tasks
@zemyblue zemyblue mentioned this pull request Apr 24, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zemyblue zemyblue zemyblue approved these changes

@dudong2 dudong2 dudong2 approved these changes

@shiki-tak shiki-tak shiki-tak approved these changes

@ulbqb ulbqb ulbqb approved these changes

@0Tech 0Tech Awaiting requested review from 0Tech 0Tech is a code owner

@loin3 loin3 Awaiting requested review from loin3

@da1suk8 da1suk8 Awaiting requested review from da1suk8

Assignees

@tkxkd0159 tkxkd0159

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove legacy REST
5 participants
@tkxkd0159 @zemyblue @dudong2 @shiki-tak @ulbqb