Skip to content

feat: pcp-messages protobuf#1266

Open
karelispanagiotis wants to merge 8 commits into
mainfrom
karelis/pcp-protobuf
Open

feat: pcp-messages protobuf#1266
karelispanagiotis wants to merge 8 commits into
mainfrom
karelis/pcp-protobuf

Conversation

@karelispanagiotis

@karelispanagiotis karelispanagiotis commented Jun 17, 2026

Copy link
Copy Markdown
Member

Introduces protobuf definitions for pcp messages to be included in the pcp-package.

@karelispanagiotis karelispanagiotis requested a review from a team as a code owner June 17, 2026 19:55
@github-actions

Copy link
Copy Markdown

No concrete merge-blocking issues found in the PR diff.

I checked the workspace wiring, proto paths, build.rs generation setup, exported Rust module, and git diff --check passed. I could not run cargo check -p orb-pcp-messages or buf lint in this sandbox because the environment is read-only and lacks buf/protoc, so CI should cover those checks.

@karelispanagiotis karelispanagiotis changed the title (feat) pcp-messages package (feat): pcp-messages protobuf Jun 17, 2026
@karelispanagiotis karelispanagiotis changed the title (feat): pcp-messages protobuf feat: pcp-messages protobuf Jun 17, 2026
@github-actions

Copy link
Copy Markdown

No concrete correctness, security, or regression issues found in the PR diff.

I could not fully verify locally: cargo check was blocked by the read-only sandbox trying to write Cargo/rustup cache state, and buf is not installed in this environment.

Comment thread pcp-messages/proto/pcp/v1/di_iris_embedding_shares.proto Outdated
Comment thread pcp-messages/proto/pcp/v1/di_iris_embeddings.proto Outdated
Comment thread Cargo.toml Outdated
Comment thread pcp-messages/README.md Outdated
@github-actions

Copy link
Copy Markdown

No concrete merge-blocking issues found in the PR diff.

I could not run cargo check -p orb-pcp-messages or buf lint in this sandbox because rustup cannot write to its home directory here, and protoc/buf are not installed. Static review only.

@github-actions

Copy link
Copy Markdown

I found no concrete merge-blocking issues in the PR diff.

I could not run full verification in this environment: cargo check hit a read-only rustup temp path, and buf/protoc are not installed outside the Nix shell here.

@github-actions

Copy link
Copy Markdown

No concrete merge-blocking issues found in the PR diff.

I couldn’t execute the new crate tests here because the read-only environment prevented rustup from creating temp files under /home/runner/.rustup; buf also isn’t installed in this runner. The schema/build wiring looks consistent with the workspace and CI’s nix develop setup.

@github-actions

Copy link
Copy Markdown

I found no concrete merge-blocking issues in the changes introduced by this PR.

Note: I couldn’t run the Rust checks locally because this sandbox is read-only/offline for the required toolchain/dependencies, so this is a static review of the diff.

@github-actions

Copy link
Copy Markdown

No concrete merge-blocking issues found in the PR diff.

I reviewed the new proto definitions, Rust prost crate wiring, workspace inclusion, and Proto CI workflow. I wasn’t able to run the Rust/buf checks locally in this read-only environment, so this is a static review.

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