Skip to content

request: switch all request types to value receivers and value returns#584

Open
bigbes wants to merge 1 commit into
masterfrom
bigbes/gh-238-request-value-receivers
Open

request: switch all request types to value receivers and value returns#584
bigbes wants to merge 1 commit into
masterfrom
bigbes/gh-238-request-value-receivers

Conversation

@bigbes
Copy link
Copy Markdown
Collaborator

@bigbes bigbes commented May 5, 2026

  • Rename Make*Requests() to New*Requests(), same for the type constructors.
  • All New*Request() constructors now return values instead of pointers; all methods use value receivers and return values, enabling immutable builder-style chaining.
  • Removed intermediate spaceRequest/spaceIndexRequest types from the tarantool package and spaceRequest from crudspace and index fields are inlined directly into each request struct. Constructors converted to composite literal style.
  • Same pattern applied to arrow/ and settings/. In box/, request types no longer embed tarantool.CallRequest — they store it via baseCallRequest and implement their own Context() returning the wrapper type.
  • See MIGRATION.md and CHANGELOG.md for the full migration guide and breaking-change note (calling a builder method without using the return value silently discards the change).

Replaces #553, which was rebased onto current master and renamed to bigbes/gh-238-request-value-receivers.

Part of #238

@bigbes bigbes force-pushed the bigbes/gh-238-request-value-receivers branch from 2c647d4 to be4af03 Compare May 5, 2026 08:02
@bigbes bigbes requested a review from oleg-jukovec May 5, 2026 11:26
@oleg-jukovec oleg-jukovec force-pushed the bigbes/gh-238-request-value-receivers branch from be4af03 to f3f2647 Compare May 16, 2026 07:56
Copy link
Copy Markdown
Collaborator

@oleg-jukovec oleg-jukovec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've rebased the PR.

But we still have New/Make different naming for value constructors in the connector:

NewFooReuqest, but MakeDatetime.

It is a bit confusing. I've suggest to rename all request constructors to MakeFooRequest OR to rename all value constructor to NewFoo.

All New*Request() constructors now return values instead of pointers. All
methods on request types use value receivers and return values, enabling
immutable builder-style chaining.

Renamed all value constructors from Make* to New* for naming consistency
across the connector: crud.Make*Request → crud.New*Request,
datetime.MakeDatetime → datetime.NewDatetime, decimal.MakeDecimal /
MakeDecimalFromString / MustMakeDecimal → decimal.NewDecimal /
NewDecimalFromString / MustNewDecimal, arrow.MakeArrow → arrow.NewArrow,
crud.MakeResult → crud.NewResult.

Removed intermediate spaceRequest, spaceIndexRequest types from the
tarantool package and spaceRequest from the crud package — space and
index fields are inlined directly into each request struct.

Converted all constructors to composite literal style.

Applied the same pattern to arrow/ and settings/ packages.

In the box/ subpackage, request types no longer embed
tarantool.CallRequest. They store it as a private field via
baseCallRequest and implement their own Context() method returning the
wrapper type.

Part of #238
@bigbes bigbes force-pushed the bigbes/gh-238-request-value-receivers branch from f3f2647 to 42cbf81 Compare May 23, 2026 12:40
@bigbes bigbes requested a review from oleg-jukovec May 23, 2026 12:44
@bigbes
Copy link
Copy Markdown
Collaborator Author

bigbes commented May 23, 2026

Fixed, ty!

Comment thread MIGRATION.md
```Go
// Before (pointer receivers, mutation in place):
req := NewSelectRequest("space")
req.Index("idx") // mutated req directly
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
req.Index("idx") // mutated req directly
req.Index("idx") // Mutated req directly.

Comment thread MIGRATION.md

// After (value receivers, must use return value):
req := NewSelectRequest("space")
req = req.Index("idx") // must reassign
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

etc

Suggested change
req = req.Index("idx") // must reassign
req = req.Index("idx") // Must reassign.

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.

2 participants