Skip to content
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

update Dockerfiles to remove the warnings #5035

Merged
merged 1 commit into from
Oct 19, 2024

Conversation

pfi79
Copy link
Contributor

@pfi79 pfi79 commented Oct 17, 2024

No description provided.

@pfi79 pfi79 requested a review from a team as a code owner October 17, 2024 18:29
@@ -48,7 +48,7 @@ RUN make orderer GO_TAGS=${GO_TAGS} FABRIC_VER=${FABRIC_VER}
###############################################################################

ARG UBUNTU_VER
FROM ubuntu:${UBUNTU_VER}
FROM ubuntu:${UBUNTU_VER:-20.04}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

22.04?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix

@@ -19,7 +19,7 @@
ARG GO_VER
ARG UBUNTU_VER

FROM ubuntu:${UBUNTU_VER} as base
FROM ubuntu:${UBUNTU_VER:-22.04} AS base
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the reasons we use variables like this is so that we only have to maintain them in one place. Setting the default value everywhere defeats the purpose of that.

Is there a compelling reason to set the default?

Copy link
Contributor Author

@pfi79 pfi79 Oct 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a compelling reason to set the default?

This is the kind of warning from docker.

https://docs.docker.com/reference/build-checks/invalid-default-arg-in-from/

 1 warning found (use docker --debug to expand):
 - InvalidDefaultArgInFrom: Default value for ARG ubuntu:${UBUNTU_VER} results in empty or invalid base image name (line 24)

But maybe that's not a good reason.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The risk from that warning is understood and pretty much nil, so I'm thinking it's acceptable to not fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok. then do we close pr or keep some of the changes?

Signed-off-by: Fedor Partanskiy <fredprtnsk@gmail.com>
Copy link
Contributor

@denyeart denyeart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now, thanks!

@denyeart denyeart merged commit 80a4e77 into hyperledger:main Oct 19, 2024
15 checks passed
@pfi79 pfi79 deleted the update-dockerfile branch October 19, 2024 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants