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
Create gitlab pipeline #534
Create gitlab pipeline #534
Changes from all commits
91128a6
9e251a8
93fc653
5262318
a24b9bb
7e3fe6d
e6c8358
5b35d10
becbdf0
68b72ca
20589ea
c6c443b
b2a0c9f
8cd5e48
0aaee60
a9175ec
0408012
9588b80
abe5dc7
95af7a8
80ce0af
0cf9290
da23d5e
99ddccd
f284330
e3809cd
c44422d
9f944f1
6cf75c9
bede6bb
a5ce28e
cf0ae9f
39adb0f
a6d6258
5b7f149
18bfe81
864b89b
ccd8f7d
51d4ca9
7f76577
4e214fb
b0163ba
4ffeeac
9b4b815
bd12ab4
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.
Why isn't the
rustfmt
component part of the base image? Seems like setting up the toolchain components should be in a single place?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.
Usually, we run
cargo fmt
from the nightly release in other repos. I'll add comment here to fix it later.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. I guess the question still stands though: wherever you install the nightly toolchain could be a good place to add the
rustfmt
component as well? And then runcargo +nightly fmt
here?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.
@dvdplm We have nightly fmt installed.
I guess you mean that you're OK with
cargo +nightly fmt
running here, right?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'd prefer
cargo fmt
from stable, butcargo +nightly fmt
works too.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.
it's not a big deal, we can add it to the CI image for stable, too.
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 added clippy and fmt to the image however need to wait while issue in substrate is fixed
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.
Same concern here.
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.
@dvdplm We have nightly clippy installed.
I guess you mean that you're OK with
cargo +nightly clippy --all-targets
running here, right?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.
Same as above, I prefer CI to use stable but yes, I'm ok running nightly clippy too. Curious though, why is clippy not included in the base image?
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.
You mean
paritytech/base-ci-linux
? It was never requested before to run a stableclippy
orfmt
.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.
do you need to use
paritytech/ci-linux
here? It's better off with a lighterparitytech/tools
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.
Agree, fixed
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.
then it maybe makes sense to remove that default image from k8s anchor.
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'd prefer to keep k8s anchor as it is so it is equal through all our
.gitlab-ci.yml
files in different repos