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

More Agoric tweaks #399

Merged
merged 5 commits into from
Feb 2, 2021
Merged

Conversation

michaelfig
Copy link
Contributor

I found these changes were useful to help make the Agoric dynamic IBC layer work correctly:

  • allow rly config show --json to dump the YAML config as JSON
  • rly tx conn should create the client if it isn't already
  • minor fix to add a missing help %s format argument to rly tx raw send
  • have nchainz default to set up the connections but not relay any paths
  • turn down the Agoric chain --log_level to warn to help prevent noise

@lgtm-com
Copy link

lgtm-com bot commented Feb 2, 2021

This pull request introduces 1 alert when merging 6c68cfe into b1360ef - view on LGTM.com

new alerts:

  • 1 for Useless assignment to local variable

@lgtm-com
Copy link

lgtm-com bot commented Feb 2, 2021

This pull request introduces 1 alert when merging d274e1e into b1360ef - view on LGTM.com

new alerts:

  • 1 for Useless assignment to local variable

Copy link
Contributor

@colin-axner colin-axner left a comment

Choose a reason for hiding this comment

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

utACK, thanks @michaelfig

Copy link
Member

@AdityaSripal AdityaSripal left a comment

Choose a reason for hiding this comment

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

utACK, thanks @michaelfig!

@@ -206,7 +206,15 @@ $ %s tx con demo-path -o 3s`, appName, appName, appName)),
return err
}

modified, err := c[src].CreateOpenConnections(c[dst], retries, to)
// ensure that the clients exist
modified, err := c[src].CreateClients(c[dst])
Copy link
Member

Choose a reason for hiding this comment

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

Should similar logic exist for CreateChannel? Where we create client and connection if they don't exist?

cc: @colin-axner

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Where we create client and connection if they don't exist?

Possibly. The current tx link subcommand does that already, FWIW.

Copy link
Contributor

Choose a reason for hiding this comment

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

no preference here, maybe it makes sense to deprecate create channels? or rename tx link? Doesn't make too much sense to maintain 2 commands that do the same thing

Copy link
Contributor

Choose a reason for hiding this comment

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

opened #400, outside the scope of this pr

@colin-axner colin-axner merged commit 2f1a812 into cosmos:master Feb 2, 2021
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