Skip to content

Conversation

@not-napoleon
Copy link
Member

This is an alternative approach to the refactoring in #127851. As discussed there, instead of adding a builder, I've just changed all the constructor invocations to explicitly pass in the various optional fields, and only kept the constructor accepting all fields.

In my opinion, this is a worse solution. It's more verbose, and it scatters the knowledge of the default values throughout the code base rather than keeping them in one place. That said, it does avoid extra object creation.

@not-napoleon not-napoleon requested a review from ivancea May 8, 2025 13:39
@not-napoleon not-napoleon added >non-issue auto-backport Automatically create backport pull requests when merged :Analytics/ES|QL AKA ESQL v8.19.0 v9.1.0 labels May 8, 2025
@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label May 8, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

@not-napoleon not-napoleon requested review from idegtiarenko and nik9000 and removed request for ivancea May 8, 2025 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/ES|QL AKA ESQL auto-backport Automatically create backport pull requests when merged >non-issue Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.19.0 v9.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants