-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Test packaged EOSIO Linux artifacts against standard dockers. #10429
Conversation
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.
LGTM
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.
Mostly just concerned with the first three comments.
@@ -0,0 +1,35 @@ | |||
#!/bin/bash |
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.
We should add set -e
here so this script fails on error.
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.
Yes, we should. I'm going to add -u
also. I did it for the test script, but I guess not the wrapper...
cat > $SCRIPT_NAME <<EOL | ||
#!/bin/bash | ||
set -eu | ||
|
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.
We should use log-folding groups to hide everything from our customers except what they care about, which is just the installation and the nodeos --full-version
. This makes what the pipeline is doing more obvious and reduces our support load.
echo '+++ :minidisc: Installing EOSIO'
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.
ok. Should I put the the nodeos --full-version
into a separate fold since its a basic test layer that nodeos runs after installed? We might put other stuff there eventually.
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.
Up to you. If you do, I would make both the installation and invocation expanded log groups (+++
).
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.
Emoji are here.
.cicd/test-package.sh
Outdated
|
||
SCRIPT_NAME='test-package.gen.sh' | ||
|
||
cat > $SCRIPT_NAME <<EOL |
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.
If we wrap the opening delimiter with single-quotes...
cat > $SCRIPT_NAME <<'EOL'
...then it won't perform shell interpolation.
While your code is already working and it might suck to change it, I think it will prevent errors for anyone who has to edit it moving forward and forgets an escape sequence. Thoughts?
I, personally, prefer a recursive solution because then all the code has syntax highlighting and lives in the same file but I guess I never had time to make the same changes in eos as I did in eos-vm.
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'm fine with changing it for readability. I avoid escaping when I can.
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.
Hmm, if we do it this way, vim stops highlighting the EOL block though... ;(
.cicd/test-package.sh
Outdated
@@ -0,0 +1,35 @@ | |||
#!/bin/bash | |||
|
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.
Consider another log folding group here to match the style of the other job steps.
echo '--- :evergreen_tree: Configuring Environment'
EOL | ||
|
||
chmod +x $SCRIPT_NAME | ||
|
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.
Consider another log folding group here to match the style of the other job steps.
echo '--- :docker: Selecting Container'
Meant to click "Request Changes". Whatever, lol
1579095
to
5deca54
Compare
Change Description
Updates for CICD to test packaged EOSIO Linux artifacts against standard dockers:
*.deb
and*.rpm
files.Change Type
Select ONE:
Testing Changes
Select ANY that apply:
Consensus Changes
API Changes
Documentation Additions