search

jaegertracing / docker-protobuf

An all-inclusive protoc Docker image for the Jaeger project
https://hub.docker.com/r/jaegertracing/protobuf
Apache License 2.0
93 stars 35 forks source link

fix bug in Dockerfile for python #22

Closed Panlichen closed 3 months ago

Panlichen commented 3 years ago

Summary

fix typo in Dockerfile to make it work with --grpc_python_out= option.

sorry for other chaos in code. ...

Changes

Notes for Reviewers

...

Release Notes

Panlichen commented 3 years ago

I work on a 20 core platform. Maybe it is better to automize this configuration, but I do not know how to do it in Dockerfile.

Yuri Shkuro notifications@github.com 于 2020年11月13日周五 01:53写道：

@yurishkuro requested changes on this pull request.

Please undo changes to unrelated files

In Dockerfile https://github.com/jaegertracing/docker-protobuf/pull/22#discussion_r522301439 :

@@ -23,12 +23,12 @@ RUN apk add --no-cache automake && \ cd /protobuf && \ ./autogen.sh && \ ./configure --prefix=/usr --enable-static=no && \

  • make -j4 && \
  • make -j4 check && \
  • make -j4 install && \
  • make -j4 install DESTDIR=/out && \
  • make -j21 && \

why does this need to change?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jaegertracing/docker-protobuf/pull/22#pullrequestreview-529343365, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADIFMSRUK4KYDUQQ7RC4OGTSPQOIDANCNFSM4TTITPDQ .

yurishkuro commented 3 years ago

I work on a 20 core platform.

The build runs on Travis-ci

Panlichen commented 3 years ago

In the new commit, I rollback the -j change. I am not familiar with Travis-ci, does the default setting for it use a four-core platform?

Panlichen commented 3 years ago

Will this tiny fix be merged?

yurishkuro commented 3 years ago

Why is it even a bug? It seems consistent with the naming of all other plugins. What is the reproducer that demonstrates that this does not work?