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

feat: add protons extension #14505

Closed
wants to merge 1 commit into from

Conversation

achingbrain
Copy link
Contributor

Adds an extension id to the registry for use by the protons protobuf encoder/decoder.

Adds an extension id to the registry for use by the [protons](https://www.npmjs.com/package/protons) protobuf encoder/decoder.
@achingbrain achingbrain requested a review from a team as a code owner October 23, 2023 13:13
@achingbrain achingbrain requested review from Logofile and removed request for a team October 23, 2023 13:13
@google-cla
Copy link

google-cla bot commented Oct 23, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

achingbrain added a commit to ipfs/protons that referenced this pull request Oct 23, 2023
Adds the capability to define protons-specific custom options that
control decoding behaviour initially around limiting the sizes of
maps and lists.

```protobuf
// import the options definition - it will work without this but some
// code editor plugins may report an error if the def can't be found
import "protons-rutime/options.proto";

message MessageWithSizeLimitedRepeatedField {
  // define the size limit - here more than 10 repeated items will
  // cause decoding to fail
  repeated string repeatedField = 1 [(protons.limit) = 10];
}
```

The defintion is shipped with the `protons-runtime` module. There is
a [pending PR](protocolbuffers/protobuf#14505)
to reserve the `1186` field ID. This should be merged first and/or
the field ID updated here if it changes due to that PR.

Fixes #113
@esrauchg esrauchg added the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Oct 23, 2023
@github-actions github-actions bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Oct 23, 2023
@achingbrain
Copy link
Contributor Author

@acozzette @fowles sorry to ping you directly but it looks like you've recently merged PRs that add extension number reservations - what needs to happen for this to be merged?

@achingbrain
Copy link
Contributor Author

@acozzette thanks for approving! It looks like there's an internal check that failed so it wasn't merged? I can't access the "Details" link, it must be some internal Google thing?

copybara-service bot pushed a commit that referenced this pull request Oct 31, 2023
COPYBARA_INTEGRATE_REVIEW=#14505 from achingbrain:patch-1 1a80ee9
FUTURE_COPYBARA_INTEGRATE_REVIEW=#14505 from achingbrain:patch-1 1a80ee9
PiperOrigin-RevId: 574304610
@acozzette
Copy link
Member

@achingbrain Sorry for the delay, I was able to get it unstuck so that it has now been submitted.

@achingbrain
Copy link
Contributor Author

That's great, thanks so much

@achingbrain achingbrain deleted the patch-1 branch November 1, 2023 08:04
achingbrain added a commit to ipfs/protons that referenced this pull request Nov 1, 2023
Adds the capability to define protons-specific custom options that control decoding behaviour initially around limiting the sizes of maps and lists.

```protobuf
// import the options definition - it will work without this but some
// code editor plugins may report an error if the def can't be found
import "protons.proto";

message Message {
  // define the size limit - here more than 10 repeated items will
  // cause decoding to fail
  repeated string repeatedField = 1 [(protons.options).limit = 10];
}
```

The definition is shipped with the `protons-runtime` module. There is a [pending PR](protocolbuffers/protobuf#14505) to reserve the `1186` field ID. This should be merged first and/or the field ID updated here if it changes due to that PR.

Fixes #113
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