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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix protobuf definitions #588

Merged
merged 4 commits into from Aug 21, 2019
Merged

Fix protobuf definitions #588

merged 4 commits into from Aug 21, 2019

Conversation

fugufish
Copy link
Contributor

馃摑 Fixes incorrect proto 3 definition

The Protobuf file is listed as using proto3, but the syntax is incorrect. It seems that protobufjs doesn't care, but protoc does. We are working on implementing the protobuf serializer for moleculer-ruby which requires protoc, and we want full parity between the packets. To do this we are actually curling the proto package from moleculerjs and using that, but the definition file is syntactically incorrect for proto3.

My real suggestion would be to create a new protocol repo, that includes the protocol document itself and the packet definition files for serializers like protobuf.

This should not be a breaking change, as it simply removes the required and optional flags (not supported in proto3).

  • Bug fix (non-breaking change which fixes an issue)

@coveralls
Copy link

coveralls commented Aug 21, 2019

Pull Request Test Coverage Report for Build 2018

  • 609 of 1487 (40.95%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-16.6%) to 75.329%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/serializers/proto/packets.proto.js 609 1487 40.95%
Totals Coverage Status
Change from base Build 1990: -16.6%
Covered Lines: 5462
Relevant Lines: 6535

馃挍 - Coveralls

@icebob
Copy link
Member

icebob commented Aug 21, 2019

Hmm, good to know, thanks!

@icebob icebob merged commit 6858316 into moleculerjs:master Aug 21, 2019
@fugufish fugufish deleted the fix-protobuf-definitions branch August 21, 2019 16:27
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

3 participants