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

CLI - channel handshakes #840

Merged
merged 3 commits into from
Jul 13, 2022
Merged

CLI - channel handshakes #840

merged 3 commits into from
Jul 13, 2022

Conversation

agouin
Copy link
Member

@agouin agouin commented Jul 13, 2022

Uses PathProcessor for channel handshakes, both automatically when channel handshake messages are detected on the relevant connections and also for the CLI channel creation.

After digging through the existing channel handshake code, the configuration never seems to be modified even in cases when returning modified: true, so I do not think a channel message subscriber will be necessary as it was for the connection handshakes.

@agouin agouin marked this pull request as ready for review July 13, 2022 03:04
@agouin agouin requested a review from DavidNix July 13, 2022 03:04
Copy link
Member

@mark-rushakoff mark-rushakoff left a comment

Choose a reason for hiding this comment

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

A couple comments that can be addressed without re-review.

}
if logLine.Aux != nil {
t.Log(logLine.Aux)
fmt.Print(logLine.Aux)
Copy link
Member

Choose a reason for hiding this comment

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

Functions associated with a particular test should log to that test rather than print to stdout. Was this change intentionally committed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah yes, meant to make a separate PR for this. the logLine.Stream and Aux is formatted for stdout. Using fmt.Print makes it print the same way a docker build would. For comparison:

t.Log:
Screenshot from 2022-07-12 19-58-30

fmt.Print:
Screenshot from 2022-07-12 19-59-21

relayer/query.go Outdated Show resolved Hide resolved
_test/test_queries.go Outdated Show resolved Hide resolved
relayer/channel.go Outdated Show resolved Hide resolved
relayer/channel.go Outdated Show resolved Hide resolved
relayer/channel.go Outdated Show resolved Hide resolved
relayer/channel.go Outdated Show resolved Hide resolved
@agouin agouin merged commit de36524 into main Jul 13, 2022
@agouin agouin deleted the andrew/channel_handshakes_cli branch July 13, 2022 18:43
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.

2 participants