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

Conform to Protobuf generation specification #97

Merged
merged 2 commits into from
Nov 11, 2021

Conversation

asahasrabuddhe
Copy link
Contributor

At the moment, when we use a plugin created using this library on Windows, we get the following error:

Warning: Generated file name "proto\\v1\\auth\\models_validator.pb.go" does not conform to the Protobuf generation specification. Note that the file name must be relative, use "/" instead of "\", and not use "." or ".." as part of the file name. Buf will continue without
error here, but please raise an issue with the maintainer of the plugin and reference https://github.com/protocolbuffers/protobuf/blob/95e6c5b4746dd7474d540ce4fb375e3f79a086f8/src/google/protobuf/compiler/plugin.proto#L122

This is because the cleanGeneratorFileName function replaces the "/" with "" on Windows. Adding filepath.ToSlash will fix this issue.

@asahasrabuddhe
Copy link
Contributor Author

@keith @ardakuyumcu @pdecks Could you please review this?

@pdecks
Copy link
Contributor

pdecks commented Oct 19, 2021

LGTM!

@asahasrabuddhe
Copy link
Contributor Author

Thanks @pdecks

@keith @ardakuyumcu Could you please take a look? I'd really appreciate this being merged so that I can stop using a fork.

@pdecks pdecks merged commit 1635a6b into lyft:master Nov 11, 2021
@pdecks
Copy link
Contributor

pdecks commented Nov 11, 2021

Thanks for being patient @asahasrabuddhe! My team recently took ownership of the repo, and I missed that this was blocking.

@asahasrabuddhe
Copy link
Contributor Author

@pdecks Could you also cut a release so we can use that in our go modules?

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.

None yet

2 participants