fix: rm dependency on 08-wasm in ibc-go #4801

charleenfei · 2023-10-02T13:09:24Z

Description

closes: #4779

Commit Message / Changelog Entry

type: commit message

see the guidelines for commit messages. (view raw markdown for examples)

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

Targeted PR against correct branch (see CONTRIBUTING.md).
Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
Code follows the module structure standards and Go style guide.
Wrote unit and integration tests.
Updated relevant documentation (docs/) or specification (x/<module>/spec/).
Added relevant godoc comments.
Provide a commit message to be used for the changelog entry in the PR description for review.
Re-reviewed Files changed in the Github PR explorer.
Review Codecov Report in the comment section below once CI passes.

colin-axner · 2023-10-02T14:21:00Z

maybe we can remove the code in testing/endpoint.go as well? But if it seems like to much work, we can do it in a followup no problem

charleenfei · 2023-10-02T14:32:29Z

maybe we can remove the code in testing/endpoint.go as well? But if it seems like to much work, we can do it in a followup no problem

absolutely, i can do it. I think the markdown links should get fixed when we merge this feature branch to main as well -- so I will not fix them in this pr if that is ok. also commented out the wasm tm tests for now instead of deleting, in ref to our planning call.

crodriguezvega

LGTM! I pushed a commit commenting out a bit of code that is also not used anymore.

crodriguezvega · 2023-10-03T11:43:29Z

Looks like we can also clean up the docker file.

colin-axner

ACK, there's some other random diffs still remaining as shown here (scroll to testing/ directory changes). I'm okay to revert any of these fixes later if needed, but also happy to get them in now

I'll need to figure the workaround to the testing pkg in #4804 for creating wasm clients

go.mod

modules/light-clients/08-wasm/types/types_test.go

modules/light-clients/08-wasm/types/update_test.go

colin-axner · 2023-10-03T11:47:22Z

testing/wasm.go

@@ -1,64 +1,63 @@
 package ibctesting


I think we can remove this file, the testing pkg should be designed in a way where light clients don't need to put their testing structures here

+1, we can delete

charleenfei · 2023-10-04T08:27:02Z

Looks like we can also clean up the docker file.

I will open an issue for this, I'd prefer to keep the changes in this PR to code cleanup/deletion

…go into fix/revert-02-client-changes

damiannolan

lgtm

damiannolan · 2023-10-04T09:21:11Z

testing/wasm.go

@@ -1,64 +1,63 @@
 package ibctesting


+1, we can delete

DimitrisJim · 2023-10-04T14:55:11Z

modules/light-clients/08-wasm/types/types_test.go

-func init() {
-	ibctesting.DefaultTestingAppInit = setupTestingApp
-}
+// func init() {


QQ: was there a failure here? We need this init func to set up the correct simapp (one in 08-wasm)

you are right, i'll add it back in the next pr. i'm just confused why the tests didn't fail on this pr run 😕

charleenfei · 2023-10-05T14:14:39Z

LGTM! I pushed a commit commenting out a bit of code that is also not used anymore.

this func was actually still being used (default init()) to create the simapp for the grandpa tests -- we probably need to think of a better way of changing this than relying on this default func

charleenfei added 2 commits October 2, 2023 15:08

revert 02-client changes

e42d403

rm extra argument

969633e

charleenfei changed the title ~~fix: revert 02 client changes~~ fix: rm dependency on 08-wasm in ibc-go Oct 2, 2023

charleenfei added 3 commits October 2, 2023 15:44

rm 08-wasm dependency from ibc-go go.mod

da2d494

comment out wasm tendermint tests

ccb0777

unused import

37b4f72

charleenfei added 3 commits October 2, 2023 16:42

rm wasm code from testing utils

701f210

linter

a5f883f

clienttypes

7eba9a4

charleenfei marked this pull request as ready for review October 2, 2023 15:36

charleenfei requested review from colin-axner, AdityaSripal, damiannolan, crodriguezvega, chatton and DimitrisJim as code owners October 2, 2023 15:36

crodriguezvega added the 08-wasm label Oct 3, 2023

comment out some extra code that was used by tendermint tests

58af513

crodriguezvega approved these changes Oct 3, 2023

View reviewed changes

crodriguezvega mentioned this pull request Oct 3, 2023

testing: add wasm tendermint client config #4209

Closed

9 tasks

colin-axner approved these changes Oct 3, 2023

View reviewed changes

colin-axner mentioned this pull request Oct 3, 2023

feat: add mockVM for wasm clients testing #4804

Merged

9 tasks

charleenfei added 2 commits October 4, 2023 10:27

delete testing code

870e7de

Merge branch 'fix/revert-02-client-changes' of github.com:cosmos/ibc-…

b96c8c6

…go into fix/revert-02-client-changes

damiannolan approved these changes Oct 4, 2023

View reviewed changes

testing/wasm.go Outdated

@@ -1,64 +1,63 @@

package ibctesting

Copy link

Member

damiannolan Oct 4, 2023

Choose a reason for hiding this comment

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

+1, we can delete

delete testing/wasm.go

1fa4fe3

charleenfei merged commit 176a497 into feat/wasm-clients Oct 4, 2023
17 of 18 checks passed

charleenfei deleted the fix/revert-02-client-changes branch October 4, 2023 09:26

charleenfei mentioned this pull request Oct 4, 2023

small fix to remove the option of generating a wasm tm client #4814

Closed

9 tasks

colin-axner mentioned this pull request Oct 4, 2023

Remove dependency on 08-wasm from ibc-go #4779

Closed

3 tasks

DimitrisJim reviewed Oct 4, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: rm dependency on 08-wasm in ibc-go #4801

fix: rm dependency on 08-wasm in ibc-go #4801

charleenfei commented Oct 2, 2023 •

edited

Loading

colin-axner commented Oct 2, 2023 •

edited

Loading

charleenfei commented Oct 2, 2023 •

edited

Loading

crodriguezvega left a comment

crodriguezvega commented Oct 3, 2023

colin-axner left a comment •

edited

Loading

colin-axner Oct 3, 2023

damiannolan Oct 4, 2023

charleenfei commented Oct 4, 2023

damiannolan left a comment

damiannolan Oct 4, 2023

DimitrisJim Oct 4, 2023

charleenfei Oct 4, 2023

charleenfei commented Oct 5, 2023

fix: rm dependency on 08-wasm in ibc-go #4801

fix: rm dependency on 08-wasm in ibc-go #4801

Conversation

charleenfei commented Oct 2, 2023 • edited Loading

Description

Commit Message / Changelog Entry

colin-axner commented Oct 2, 2023 • edited Loading

charleenfei commented Oct 2, 2023 • edited Loading

crodriguezvega left a comment

Choose a reason for hiding this comment

crodriguezvega commented Oct 3, 2023

colin-axner left a comment • edited Loading

Choose a reason for hiding this comment

colin-axner Oct 3, 2023

Choose a reason for hiding this comment

damiannolan Oct 4, 2023

Choose a reason for hiding this comment

charleenfei commented Oct 4, 2023

damiannolan left a comment

Choose a reason for hiding this comment

damiannolan Oct 4, 2023

Choose a reason for hiding this comment

DimitrisJim Oct 4, 2023

Choose a reason for hiding this comment

charleenfei Oct 4, 2023

Choose a reason for hiding this comment

charleenfei commented Oct 5, 2023

charleenfei commented Oct 2, 2023 •

edited

Loading

colin-axner commented Oct 2, 2023 •

edited

Loading

charleenfei commented Oct 2, 2023 •

edited

Loading

colin-axner left a comment •

edited

Loading