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 spans support to CodedInputStream #7

Conversation

mkosieradzki
Copy link

As agreed in protocolbuffers#4988

TeBoring and others added 15 commits July 26, 2018 13:52
* Add conformance test for php c back

php c extension has different result for conformance test for different
php version and architecture. Try to add conformance back for php 7.1 c extension first.

* Disable conformance test for c extension on 32-bit architecture
32-bit and 64-bit have different failing tests

* Fix typo
* Give a unique category to each test.

This change introduce a TestCategory enum to ConformanceRequest. Existing tests
are divided into three categories: binary format test, json format test and json
format (ignore unknown when parsing) test. For the previous two categories, there
is no change to existing testee programs. For tests with the last category, testee programs
should either enable ignoring unknown field during json parsing or skip the test.

* Fix python test

* Fix java

* Fix csharp

* Update document

* Update csharp generated code
I made a few small fixes to the documentation related to publishing
protoc artifacts:
- The target directory for Mac should be called osx instead of macos.
- There needs to be a directory for aarch_64.
- We need to avoid calling "mvn clean" inside the protoc-artifacts
  directory, since that will delete the contents of the target/
  subdirectory.
I have added the xsrpcj RPC implementation for java
fix third_party/benchmark submodule init
… set (protocolbuffers#4825)

* Ignore unknown enum received in json when ignoreUnknownFields flag is set.
tests: fix link failure and stack overflow on Mingw w64
Exposed span-variants of CodedInputStream APIs
Added possibility to create CodedInputStream from ReadOnlySequence and ReadOnlySpan
Exposed Wrapped-version of Read APIs for future use by an updated Codegen
Exposed PushLimit/PopLimit/ReachedLimit/BeginReadNested/EndReadNested to allow more efficient inlining parsing loops by code generator
Changed ReadFloat to use Int32BitsToSingle approach
Resolved ambiguity in documentation crefs
Cleaned up duplicate after merge.
Added missing [SecurityCritical] attribute
@jtattermusch
Copy link
Owner

@mkosieradzki looks like the merge has been done incorrectly (there's extra changes by other authors from upstream/master that are showing up in the diff and we don't want that).

What I think is the right approach:

  • start with clean checkout of jtattermusch:span_support_experimental
  • git cherry-pick commits from Add spans support protocolbuffers/protobuf#4988 that you want to include in this PR and resolve conflicts (e.g. the ByteString changes are already in this branch).
  • don't use git merge as it obfuscates the commit history and brings unwanted third-party commits to the PR log (it looks like that you did git merge spans-patches and created a new PR)

@mkosieradzki
Copy link
Author

I know. I thought you will update your branch sometimes to the latest commit from master - to decrease the risk of conflicts later, but OK I can do manual rebase, but we need to fix your earlier PRs because they do not handle SecurityCritical and should have not been accepted yet.

@mkosieradzki
Copy link
Author

Closed in favor of #8

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.

7 participants