Clean up NetRPC remnants: dead code, stale comments, docs#3
Conversation
- Remove dead protocol switch in server.go that assigned ProtocolGRPC in both arms, replacing it with direct assignment - Update AllowedProtocols comment in client.go to clarify that only gRPC is supported (remove legacy reasons and new protocols references) - Update reattach protocol default comment in client.go from net/rpc backwards compatibility to gRPC Simplifies code and clarifies that gRPC is the sole supported protocol. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- server.go: remove stale net/rpc comment in graceful shutdown path - examples/negotiated/main.go: remove "netrpc" from error message - examples/negotiated/README.md: remove stale netrpc usage example - examples/grpc/README.md: remove plugin-go-netrpc section (directory removed) - README.md: update architecture and usage docs to gRPC-only - docs/internals.md: update PROTOCOL field description, drop netrpc mention - docs/guide-plugin-write-non-go.md: same PROTOCOL field update - docs/extensive-go-plugin-tutorial.md: fix server struct comment referencing net/rpc Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR removes remaining NetRPC-era dead code and stale references, aligning the codebase, examples, and documentation around gRPC as the only supported transport.
Changes:
- Simplifies server protocol selection by removing a dead protocol switch and setting
ProtocolGRPCdirectly. - Updates client comments around
AllowedProtocolsand reattach protocol defaults to reference gRPC only. - Removes NetRPC references from examples and documentation.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
server.go |
Removes dead protocol-selection loop; updates shutdown comment to be gRPC-only. |
client.go |
Clarifies comments that default/reattach behavior is gRPC-only. |
examples/negotiated/main.go |
Updates user-facing error text to indicate only grpc is accepted. |
examples/negotiated/README.md |
Updates walkthrough to use gRPC-only examples and expected output. |
examples/grpc/README.md |
Removes the NetRPC plugin example section. |
docs/internals.md |
Updates protocol field description to state only grpc is supported. |
docs/guide-plugin-write-non-go.md |
Updates protocol field description to state only grpc is supported. |
docs/extensive-go-plugin-tutorial.md |
Updates a comment to remove NetRPC wording (but see review comment about correctness). |
README.md |
Updates architecture/usage wording to describe gRPC-only operation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * `PROTOCOL` is the named protocol that the connection will use. The only | ||
| supported value is "grpc". This is the protocol that the plugin uses to | ||
| speak to the host process. |
There was a problem hiding this comment.
The handshake documentation still describes a 5-field line (CORE|APP|NETWORK|ADDR|PROTOCOL), but the server currently prints additional fields (server cert and gRPC broker multiplexing support) and the client parser treats PROTOCOL and later fields as optional. Please update the documented handshake format/example to match what the client expects (at least 4 fields, with optional PROTOCOL, server cert, and mux flag), so non-Go plugin implementations can interoperate correctly.
| * `PROTOCOL` is the named protocol that the connection will use. The only | ||
| supported value is "grpc". This is the protocol that the plugin uses to | ||
| speak to the host process. |
There was a problem hiding this comment.
This guide still documents the handshake line as exactly 5 fields and gives an example with only those fields, but go-plugin’s server/client handshake now supports additional optional fields (server cert and gRPC broker multiplexing support) and even allows PROTOCOL to be omitted (defaults to gRPC). Please update the handshake format description/example here to reflect the current wire format and which fields are required vs optional.
| ~~~go | ||
| // Here is the RPC server that GreeterRPC talks to, conforming to | ||
| // the requirements of net/rpc | ||
| // Here is the gRPC server that GRPCClient talks to. |
There was a problem hiding this comment.
This comment says this is a gRPC server, but the example immediately below implements a net/rpc-style handler (Greet(args interface{}, resp *string) error) and the type is named GreeterRPCServer. Either rename/adjust this section to be truly gRPC (and show the gRPC service implementation), or change the comment to describe an RPC (net/rpc) server to avoid misleading readers.
| // Here is the gRPC server that GRPCClient talks to. | |
| // Here is the RPC server that GRPCClient talks to. |
Summary
server.gothat assignedProtocolGRPCin both arms; replace with direct assignmentAllowedProtocolscomment inclient.goto clarify gRPC is the sole supported protocol; remove legacy net/rpc referencesclient.goto reference gRPC (not net/rpc backwards compatibility)server.go,examples/negotiated/main.go, example READMEs,README.md,docs/internals.md,docs/guide-plugin-write-non-go.md,docs/extensive-go-plugin-tutorial.mdSkipLogParsingperf option (PR #352), elevated panic log level (PR #292),GRPCBroker.DialWithOptions(PR #257), safe PID reattach (PR #320)Test plan
go build ./...passesgo test ./...passes (ok github.com/GoCodeAlone/go-plugin,ok github.com/GoCodeAlone/go-plugin/internal/cmdrunner)🤖 Generated with Claude Code