-
Notifications
You must be signed in to change notification settings - Fork 2.7k
check polkadot for companion pull requests #5262
Conversation
.maintain/gitlab/check_polkadot.sh
Outdated
SUBSTRATE_PATH=$(pwd) | ||
|
||
# Clone the current Polkadot master branch into ./polkadot. | ||
git clone --depth 1 https://gitlab.parity.io/parity/polkadot.git |
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.
Why does it clone from gitlab?
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 didn't change that. It probably won't be faster since the runners do not host the repo. will change to github, thanks.
f041736
to
c855ff4
Compare
I will remove the companion pr tag (it's just a random pr in the polkadot repo) after approval. |
.maintain/gitlab/check_polkadot.sh
Outdated
cargo update | ||
|
||
# Check whether Polkadot pr or master branch builds with this Substrate commit. | ||
time cargo check |
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 think we should run cargo test
.
While this would find compilation problems, it would not find behavior changes.
.maintain/gitlab/check_polkadot.sh
Outdated
mkdir .cargo | ||
# echo "paths = [ \"$SUBSTRATE_PATH\" ]" > .cargo/config | ||
# see https://doc.rust-lang.org/cargo/reference/overriding-dependencies.html | ||
echo "[patch.\"https://github.com/paritytech/substrate\"]\nsubstrate = { path = \"$SUBSTRATE_PATH\" }" > .cargo/config |
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.
This doesn't work
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 don't understand, can you pls explain?
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.
the ci pipeline is completely green for this branch: https://gitlab.parity.io/parity/substrate/pipelines/84173
(i just pushed a tag along with it before and that's why there are tag jobs that failed)
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 pushed a test commit changing a function that is used in Polkadot and the job executed successfully. This shows that this is not working.
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 it is not too time consuming for you, would you mind explaining why?
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.
When you want to "patch" a crate, you need to list it. But you are only listing the substrate
crate, which is the name of the substrate node. This crate is not even used by Polkadot.
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 see, thanks, code updated resp reverted.
Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>
ac86b4d
to
c2ce18e
Compare
c2ce18e
to
b29a323
Compare
@gabreal looks good :) I removed my test commit. |
* check_polkadot: move to external script * check_polkadot: check for companion pr Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> Co-authored-by: Bastian Köcher <git@kchr.de>
This pr will enable the
check_polkadot
ci job to check for a companion pr string in the description.polkadot companion: paritytech/polkadot#928