-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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.
- Please add the metadata to https://github.com/paritytech/polkadot/blob/1c0c709a92de263dcf7edadc7c96f2eb847b3c38/scripts/docker/polkadot/manual.Dockerfile and mention a
scripts/docker/polkadot/README.md
there.
Metadata should be quite similar to the same inci.
andrelease.
Dockerfiles, just mention the type of the image in the description and have different links. - please decide something stable with the naming convention (this is a first time we apply it, and it's not yet really written). So, where to put
ci/manual/release/debug
before the name or not (collator_ci
vsci_collator
), what to use in which case - underscore or hyphen (ci_debug
vsstaking**_**miner**-**ci
). And it would be a good help to write this naming so it would go to the fresh policy document.
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 think the naming of image is appropriate.
For instance an image called ci-debug
could belong to any project and do anything...
I commented about the naming in the issue that initiated this PR.
also commented to the initial issue |
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.
minor issue with one image name
Sorry for the nits but I think this PR is the place set those. Multiple files vs ARGDo we really want to have NamingI think we still have a mix in the names. For the |
@chevdor this all sounds good, thank you for your help.
I suggest we take the first approach and see if it's good enough. |
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 made quite a bunch of suggestion to help with the maintenance of those images in the future.
io.parity.image.source="https://github.com/paritytech/polkadot/blob/master/scripts/docker/polkadot/polkadot_builder.Dockerfile" \ | ||
io.parity.image.documentation="https://github.com/paritytech/polkadot/scripts/docker/polkadot/README.md" | ||
|
||
ARG PROFILE=release |
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.
Not a blocker but I ended up removing this ARG in other images as I don't think it makes much sense to build anything but a release version.
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.
Yeah, totally, I think it was there more for explicity.
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 initially used that when I made the first images thinking debug
image could be useful but that never happened to be really the case.
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, thanks for the big work !
LABEL io.parity.image.authors="devops-team@parity.io" \ | ||
io.parity.image.vendor="Parity Technologies" \ | ||
io.parity.image.title="builder" \ | ||
io.parity.image.description="This is the build stage for Polkadot. Here we create the binary." \ |
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 idea behind it was a policy-as-a-code in future that would check that every Dockerfile has metadata. But good point on readability, makes sense to make an exclusion for the files with the specific *builder*
in the name for such policy.
LABEL io.parity.image.authors="devops-team@parity.io" \ | ||
io.parity.image.vendor="Parity Technologies" \ | ||
io.parity.image.title="builder" \ | ||
io.parity.image.description="This is the build stage for Polkadot. Here we create the binary." \ |
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 OK to have simple explanation comments instead of the full metadata in such files.
now you should wait until #3807 gets merged |
As a side note (I can open another issue if you prefer), it would be super convenient to have an injected docker image where the binary is built with all features. That would help with |
@chevdor in another PR please |
New issue about a new image with |
4c3d779
to
2e74764
Compare
Co-authored-by: Chevdor <chevdor@users.noreply.github.com>
Co-authored-by: Chevdor <chevdor@users.noreply.github.com>
Co-authored-by: Chevdor <chevdor@users.noreply.github.com>
Co-authored-by: Chevdor <chevdor@users.noreply.github.com>
Co-authored-by: Chevdor <chevdor@users.noreply.github.com>
Co-authored-by: Chevdor <chevdor@users.noreply.github.com>
Co-authored-by: Chevdor <chevdor@users.noreply.github.com>
Co-authored-by: Chevdor <chevdor@users.noreply.github.com>
Co-authored-by: Chevdor <chevdor@users.noreply.github.com>
Co-authored-by: Chevdor <chevdor@users.noreply.github.com>
Co-authored-by: Chevdor <chevdor@users.noreply.github.com>
2e74764
to
f5e84fa
Compare
* master: feat: measured oneshots (#3902) remove `AllSubsystems` and `AllSubsystemsGen` types (#3874) Companion for Substrate#9867 (#3938) Substrate Companion for #9552 (#3834) CI: run disputes tests (#3962) Bump parity-scale-codec from 2.3.0 to 2.3.1 (#3959) approval-voting: populate session cache in advance (#3954) Bump libc from 0.2.102 to 0.2.103 (#3950) fix master (#3955) Docker files chore (#3880) Bump nix from 0.19.1 to 0.20.0 (#3587) remove connected disconnected state, 3rd attempt (#3898) fix flaky chain-selection tests (#3948) Add benchmarking for parachain runtime initializer pallet (#3913) minor chore changes (#3944) disputes: reject single-sided disputes (#3903)
* master: (52 commits) Companion for substrate PR#9890 (#3961) Bump version, tx_version and spec_version in prep for v0.9.11 (#3970) Fix master compilation (#3977) Make most XCM APIs accept an Into<MultiLocation> where MultiLocation is accepted (#3627) fix disputes tests (#3974) Drop availability only for candidates that lose disputes (#3973) revert +1 change to be on the safer side (#3972) paras_inherent: reject only candidates with concluded disputes (#3969) feat: measured oneshots (#3902) remove `AllSubsystems` and `AllSubsystemsGen` types (#3874) Companion for Substrate#9867 (#3938) Substrate Companion for #9552 (#3834) CI: run disputes tests (#3962) Bump parity-scale-codec from 2.3.0 to 2.3.1 (#3959) approval-voting: populate session cache in advance (#3954) Bump libc from 0.2.102 to 0.2.103 (#3950) fix master (#3955) Docker files chore (#3880) Bump nix from 0.19.1 to 0.20.0 (#3587) remove connected disconnected state, 3rd attempt (#3898) ...
Renamed docker files with unified naming convention, adjusted .gitlab-ci.yml and documentation.
Moved docker folder to script/docker/polkadot, adjusted scripts and documentation.
Closes https://github.com/paritytech/ci_cd/issues/197