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

Separate proto3 and proto2 standard types. #18

Merged
merged 5 commits into from
Mar 16, 2017
Merged

Separate proto3 and proto2 standard types. #18

merged 5 commits into from
Mar 16, 2017

Conversation

jkukul
Copy link
Contributor

@jkukul jkukul commented Mar 5, 2017

Currently, --include_std_types parameter doesn't work when compiling proto2 files containing standard types (e.g. protobuf's Options). The problem is that descriptor.proto in proto3 is not backward compatible and thus proto2 compiler cannot compile it.
In 062fd0c I'm proposing keep proto2 and proto3 proto files separately.

On the way, I added some patches to facilitate further development.

  • 34fcc82 - right now running the test suite mvn test always seem to succeed. When making changes, would nice if mvn test facilitated regression testing.
  • 44cbf9c - I wanted to add a similar directory to include and I realised that with the current setup one has to always remember to modify the assembly.xml file.

So that it's automatically packaged.

And there has to be no boilerplate of configuring it in assembly.xml
@os72
Copy link
Owner

os72 commented Mar 8, 2017

Ack. Thank you. I see your solution with resources/include, will have a look. What else would you want to include?

Proto3's descriptor.proto is not backward compatible.
@jkukul jkukul changed the title Project improvements. Separate proto3 and proto2 standard types. Mar 9, 2017
@jkukul
Copy link
Contributor Author

jkukul commented Mar 9, 2017

@os72 I addded another commit and adjusted the PR description.

@os72
Copy link
Owner

os72 commented Mar 14, 2017

Thank you. I've been busy, will look to merge soon

@os72 os72 merged commit 9cf3b7e into os72:master Mar 16, 2017
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