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

add instructions to install protoc in contributing.md #16904

Closed
moficodes opened this issue Nov 9, 2023 · 5 comments
Closed

add instructions to install protoc in contributing.md #16904

moficodes opened this issue Nov 9, 2023 · 5 comments

Comments

@moficodes
Copy link
Member

What would you like to be added?

add links / instruction to install protoc for setting up the dev environment.

it should also mention that protoc needs to be version 3.20.3 https://github.com/etcd-io/etcd/blob/main/scripts/genproto.sh#L27

Why is this needed?

make verify fails in protoc is not available in the system or the version is anything other than 3.20.3

@moficodes moficodes changed the title add instructions to install protoc in contributing add instructions to install protoc in contributing.md Nov 9, 2023
@vivekpatani
Copy link
Contributor

I think it's to produce reproducible builds by pinning protoc generation to a specific version.

Source

if [[ $(protoc --version | cut -f2 -d' ') != "3.20.3" ]]; then
  echo "could not find protoc 3.20.3, is it installed + in PATH?"
  exit 255
fi

@moficodes
Copy link
Member Author

Thats reasonable. But the contributing.md does not mention protoc being a requirement. So when make verify is run it will fail the test.

@jmhbnz
Copy link
Member

jmhbnz commented Nov 9, 2023

Thanks for raising this @moficodes - I think it makes sense to add a bullet point under https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#option-1---manually-setup-local-environment to cover installing the projects pinned version of protoc. If you have time please raise a pr 🙏🏻

@moficodes
Copy link
Member Author

Please assign this to me.

There is also some other requirements for this that I can add to the same pr.

e.g - yamllint

@serathius
Copy link
Member

Would be great enhancement to add a make install-protoc install-yamllint commands

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants