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

Remove all genproto.sh files #94

Merged
merged 1 commit into from
Jun 3, 2022
Merged

Remove all genproto.sh files #94

merged 1 commit into from
Jun 3, 2022

Conversation

mic-max
Copy link
Contributor

@mic-max mic-max commented Jun 2, 2022

This change is towards the goal of moving the generation of the generated protobuf files to the Dockerfile and not checking them into the repository.

These files are not used by anything.

Changes

Remove all genproto.sh files

@mic-max mic-max requested a review from a team as a code owner June 2, 2022 22:41
@julianocosta89
Copy link
Member

LGTM and I have only one note (not a blocker).
Removing the genproto.sh files will not impact the services, but it may impact on ppl trying to build/compile the project locally.
Hence, it would be good to have a README, explaining how to compile the proto files per service.
Something like @mic-max have done in this PR already: https://github.com/open-telemetry/opentelemetry-demo-webstore/blob/300001bb6f895bffca5910eccaa7569285308f8e/src/recommendationservice/README.md

@mic-max
Copy link
Contributor Author

mic-max commented Jun 3, 2022

By build/compile the project locally I guess you also mean without using docker, right? for example, they run go build and it complains about the missing files

In follow-up PRs we could include this instruction in the service READMEs like you linked the example where I did so 👍

@cartersocha cartersocha merged commit 7f540ff into open-telemetry:main Jun 3, 2022
@mic-max mic-max deleted the del-genproto-scripts branch June 3, 2022 18:59
GaryPWhite pushed a commit to wayfair-contribs/opentelemetry-demo that referenced this pull request Jun 30, 2022
@GaryPWhite GaryPWhite mentioned this pull request Jun 30, 2022
2 tasks
jmichalak9 pushed a commit to jmichalak9/opentelemetry-demo that referenced this pull request Mar 22, 2024
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.

3 participants