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 mypy support #288

Merged
merged 13 commits into from
Sep 2, 2022
Merged

Add mypy support #288

merged 13 commits into from
Sep 2, 2022

Conversation

maxjakob
Copy link
Contributor

@maxjakob maxjakob commented Mar 4, 2022

Enable the protoc-gen-mypy plugin through https://github.com/nipunn1313/mypy-protobuf.

Addresses #81.

@maxjakob
Copy link
Contributor Author

maxjakob commented Mar 4, 2022

Hello @ido-namely ! 👋
Could you or somebody from the Namely team trigger the build for this? Would be great to get this in.
Thanks for maintaining this really useful project! :)

build.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@ido-namely ido-namely left a comment

Choose a reason for hiding this comment

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

Thank you @maxjakob for the contribution!
I left a few comments but the PR looks really good!

I also suggest to add tests in https://github.com/namely/docker-protoc/blob/master/all/test/all_test.go, should be straightforward.

Might be better if I let the build run after that.

Thanks!

Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
all/entrypoint.sh Outdated Show resolved Hide resolved
variables.sh Show resolved Hide resolved
Copy link
Contributor Author

@maxjakob maxjakob left a comment

Choose a reason for hiding this comment

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

Thanks so much for the feedback @ido-namely! I addressed most of your comments.

Dockerfile Outdated Show resolved Hide resolved
@maxjakob maxjakob requested review from ido-namely and removed request for ido-namely September 2, 2022 15:08
Copy link
Contributor

@ido-namely ido-namely left a comment

Choose a reason for hiding this comment

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

almost there :)

all/test/all_test.go Show resolved Hide resolved
all/test/all_test.go Show resolved Hide resolved
.github/renovate.json5 Outdated Show resolved Hide resolved
@maxjakob maxjakob requested review from ido-namely and removed request for abe545 and nycdotnet September 2, 2022 17: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.

3 participants