refactor!: rename Response->Message and remove Response types #114

2026-01-05T23:27:02Z

nemith commented

2026-01-05 23:27:02 +00:00

(Migrated from github.com)

While working on the new Notification support it was obvious that a Reponse being a stream of the returning object is the same for<notification> as well as <rpc-reply>' messages. Message` is a much better name for the objects comming from a NETCONF server.

So while we are still making some breaking changes lets do it right.

Also with Response gone Request is looking very useless. Lets use RPC directly.

Also added a (s *Session) Prepare(*RPC) *RPC method which will take a bare RPC and make it the exact version that will be sent on the wire (i.d with a message-id if one was missing). This is optional but needed for an upcoming CLI I will be creating.

While working on the new Notification support it was obvious that a Reponse being a stream of the returning object is the same for`<notification>` as well as `<rpc-reply>' messages. `Message` is a much better name for the objects comming from a NETCONF server. So while we are still making some breaking changes lets do it right. Also with `Response` gone `Request` is looking very useless. Lets use `RPC` directly. Also added a `(s *Session) Prepare(*RPC) *RPC` method which will take a bare RPC and make it the exact version that will be sent on the wire (i.d with a message-id if one was missing). This is optional but needed for an upcoming CLI I will be creating.

copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed

2026-01-05 23:35:08 +00:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull request overview

This PR performs a significant refactoring of the NETCONF library's core messaging types to better support both RPC replies and notifications. The main change renames Response to Message to reflect that NETCONF streams can contain both <rpc-reply> and <notification> messages. Additionally, it removes the Request wrapper type in favor of using RPC directly, and adds a Prepare() method for explicitly preparing RPCs with message IDs.

Key changes:

Renamed Response type to Message with encapsulated reader and message-id fields
Refactored Session.Do() to accept *RPC directly instead of *Request, with automatic message-id assignment
Added Session.Prepare() method to explicitly prepare RPCs with message-ids for use cases like CLI tools

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
session.go	Updated Do/Exec/Close methods to use RPC directly; added Prepare() and nextMessageID() helper methods; updated pendingReq to use msg channel instead of reply
msg.go	Renamed Response to Message with private reader/messageID fields and new accessor methods; added RPCOption interface, NewRPC() constructor, and RPC.Clone() method; kept Request type for backward compatibility

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

## Pull request overview This PR performs a significant refactoring of the NETCONF library's core messaging types to better support both RPC replies and notifications. The main change renames `Response` to `Message` to reflect that NETCONF streams can contain both `<rpc-reply>` and `<notification>` messages. Additionally, it removes the `Request` wrapper type in favor of using `RPC` directly, and adds a `Prepare()` method for explicitly preparing RPCs with message IDs. Key changes: - Renamed `Response` type to `Message` with encapsulated reader and message-id fields - Refactored `Session.Do()` to accept `*RPC` directly instead of `*Request`, with automatic message-id assignment - Added `Session.Prepare()` method to explicitly prepare RPCs with message-ids for use cases like CLI tools ### Reviewed changes Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments. | File | Description | | ---- | ----------- | | session.go | Updated Do/Exec/Close methods to use RPC directly; added Prepare() and nextMessageID() helper methods; updated pendingReq to use msg channel instead of reply | | msg.go | Renamed Response to Message with private reader/messageID fields and new accessor methods; added RPCOption interface, NewRPC() constructor, and RPC.Clone() method; kept Request type for backward compatibility | --- 💡 <a href="/nemith/netconf/new/main/.github/instructions?filename=*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.

msg.go

					
				@ -28,0 +59,4 @@

						MessageID:  r.MessageID,

						Attributes: slices.Clone(r.Attributes),

						Operation:  r.Operation,

					}

The Clone method performs a shallow copy of the Operation field. If Operation contains pointers or reference types, modifications to the cloned RPC's Operation will affect the original. Consider documenting this behavior or implementing a deep copy if the Operation field needs to be fully independent

msg.go

					
				@ -158,3 +199,4 @@

				type Hello struct {

					XMLName      xml.Name `xml:"urn:ietf:params:xml:ns:netconf:base:1.0 hello"`

					SessionID    uint64   `xml:"session-id,omitempty"`

					Capabilities []string `xml:"capabilities>capability"`

Typo in documentation: "dones't" should be "doesn't"

// the message doesn't have a message-id attribute (for example it is a

Typo in documentation: "dones't" should be "doesn't" ```suggestion // the message doesn't have a message-id attribute (for example it is a ```

session.go

					
				@ -371,3 +388,4 @@

					raw, err := io.ReadAll(msg)

					if err != nil {

						return fmt.Errorf("failed to read reply: %w", err)

					}

The comment should be updated to reflect that this is a higher-level method compared to the old implementation. It now automatically assigns a message-id if one is missing, which is different behavior from before

// Do is a higher-level helper that sends the given rpc message and waits for
// the response message. If the rpc has no message-id, Do will first call
// Prepare to clone it and assign a message-id automatically.

The comment should be updated to reflect that this is a higher-level method compared to the old implementation. It now automatically assigns a message-id if one is missing, which is different behavior from before ```suggestion // Do is a higher-level helper that sends the given rpc message and waits for // the response message. If the rpc has no message-id, Do will first call // Prepare to clone it and assign a message-id automatically. ```

session.go

The Do method modifies the input parameter rpc by reassigning it when MessageID is empty. This can be confusing because the modification only affects the local variable, not the caller's copy. Consider one of these approaches:

Document that the method may internally clone the RPC if needed but does not modify the caller's RPC
Always clone the RPC before use to avoid any confusion about mutation
Return an error if MessageID is empty and require callers to use Prepare() explicitly

The Do method modifies the input parameter `rpc` by reassigning it when MessageID is empty. This can be confusing because the modification only affects the local variable, not the caller's copy. Consider one of these approaches: 1. Document that the method may internally clone the RPC if needed but does not modify the caller's RPC 2. Always clone the RPC before use to avoid any confusion about mutation 3. Return an error if MessageID is empty and require callers to use Prepare() explicitly

session.go

The documentation comment should be updated to reflect the new parameter names. It still refers to req and resp but the parameters are now op and reply

// Exec issues an RPC message with `op` as the body and decodes the response into
// a pointer at `reply`. Reply must include the full <rpc-reply> structure.

The documentation comment should be updated to reflect the new parameter names. It still refers to `req` and `resp` but the parameters are now `op` and `reply` ```suggestion // Exec issues an RPC message with `op` as the body and decodes the response into // a pointer at `reply`. Reply must include the full <rpc-reply> structure. ```

nemith (Migrated from github.com) reviewed

2026-01-05 23:52:06 +00:00

session.go

					
				@ -371,3 +388,4 @@

					raw, err := io.ReadAll(msg)

					if err != nil {

						return fmt.Errorf("failed to read reply: %w", err)

					}