Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Cleanup docs #87
Cleanup docs #87
Changes from 3 commits
f0bdc1b
16ff0f7
6897c9b
5de6c90
aeab864
a3e8bb7
67c607b
cadc142
9811ffa
439b8da
975ada1
dde09fb
4e46f28
8efe96f
180b629
d2a4aff
d03a697
e1b9cb0
1bcb0a9
5b0adf5
5473492
298c6b3
72fbea7
50d23db
0e3157c
eec9825
ead9506
65a912c
80ca25e
5510d59
765b24d
5992f19
42fbae9
63c45d9
45fcabb
24dbbca
28ff12c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wondered if the instructions just below could be simplified to just one command:
docker exec -it relayer_run /bin/bash -c "set -a; source vars.sh; ./integration-tests/basic_send_receive.sh"
. But, when I tried, I found that those instructions don't work.For one thing, when I try to run the script, I get:
Because that script path needs to be prepended with
scripts/local/
.But then when I run the command with the proper path, I get:
While we've mounted the scripts from the teleporter repo into the relayer container, via the volume declaration in the docker compose file,
foundry
isn't available because the awm-relayer'sDockerfile
doesn't install it.After going into the container and installing foundry, via the same command as the
RUN
statement that does that in teleporter'sDockerfile
, I can exit the container back to the host system and then successfully run that succinct command quoted at the beginning of this message.I know this is a whole can of worms to open in this PR of doc changes, it did feel like it fell under the purview of "cleanup docs." :)
I discovered that the same command can be successfully run via the
local_network_run
container instead of therelayer_run
one, and since that container already hasforge
installed, maybe we should use that instead?Sorry to be so long winded haha, but I think this whole comment boils down to just one statement: Delete the code block below and accept the following suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for digging into this. I'd actually prefer to keep the commands as is, that shell in the container persists after running
basic_send_receive.sh
, in case the user wants to run other tests or inspect the state of the chains manually. Providing a command via-c
won't give the user an interactive shell, if I'm not mistaken.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that that approach is better, so thank you for pushing back on my suggestion.
But, one remaining bit in this thread here: the
relayer_run
container does not have foundry installed, and I believe the easiest fix would be to change the command to reference thelocal_network_run
container instead.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! Corrected now.