-
Notifications
You must be signed in to change notification settings - Fork 588
build: warning msg on deprecated flags #810
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
Conversation
28c3972 to
cc0ecbb
Compare
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.
Squash/Security-opt/isolation should be deprecated. Replacement for squash is to squash inside build (eg. in Dockerfile with additional stage). For Security-opt, RUN --security=insecure should be used. For --net=custom alternative is to set it on buildx create. I think this one could remain error.
For cgroup-parent I'm not sure. Maybe let's just implement it although it is a bad design. (edit: maybe implement and leave it as hidden?)
From quick overview, the rest can remain hidden as they were. No point to show these messages for flags that don't change build outcome and when we are not sure if they will fit in design or not.
For formatting, no capitalized messages. The logrus default seems fine to me.
cc0ecbb to
68037af
Compare
Ok sgtm
Yeah implement and keep it hidden lgtm.
Done
Sure
You mean with the default timestamp included so revert my changes? |
commands/build.go
Outdated
|
|
||
| flags.StringVar(&ignore, "cgroup-parent", "", "Optional parent cgroup for the container") | ||
| flags.MarkHidden("cgroup-parent") | ||
| flags.SetAnnotation("cgroup-parent", "flag-warn", []string{"cgroup-parent is not implemented."}) |
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 guess plan was to implement this so set it in frontend-opt instead.
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
1e1d7fe to
c12379d
Compare
|
|
||
| ```console | ||
| $ docker run -d --name jaeger -p 6831:6831/udp -p 16686:16686 jaegertracing/all-in-one | ||
| $ docker buildx create --name builder --driver docker-container --driver-opt network=host --driver-opt env.JAEGER_TRACE=localhost:6831 --use |
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.
Instead of --driver-opt network=host we could use the jaeger container IP directly. Or maybe there is a parameter for jaeger to listen 6831 on gateway IP. This can be done later as well.
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
c12379d to
25d2f73
Compare

Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.

Relates to moby/moby#40379
Add warning message for deprecated flags via pflag annotations:
WARN[000].Signed-off-by: CrazyMax crazy-max@users.noreply.github.com