Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enforce CommandResultBuilder #1518

Open
amorton opened this issue Oct 9, 2024 · 0 comments
Open

Enforce CommandResultBuilder #1518

amorton opened this issue Oct 9, 2024 · 0 comments
Labels
Code Refactor Refactoring to change structure (medium to large)

Comments

@amorton
Copy link
Contributor

amorton commented Oct 9, 2024

code in #1497 added a CommandResultBuilder that makes it easier and more reliable to create the CommandResult

However we still need to:

  • convert the CommandResult to a class, move the builder to be inner, and protect the constructor so it can only be created using the builder.
  • make the resolvers create the CommandResultBuilder in inject into the operations (I think) or do something else so we are reliably detecting if debug mode is set. See CommandResult.singleDocumentBuilder() for comments.
  • we may not need / want to though, the OperationAttempt framework uses a page builder that already uses this CommandResultBuilder so it may not be necessary to make changes only for collections right now.

this ticket is to remember, and decide if we want to make the change
amorton added a commit that referenced this issue Oct 9, 2024
@amorton
Use WarningException, expand CommandResultBuilder
756f615 
Changes the warnings associated commands to use the new ApiExcpetions,
and `warnings` in the return status now returns a list of error object V2.

To do that needed to improve the way CommandResult was built, so it always
had a status map so the CommandProcessor could append a warning. To do that
expanded the CommandResultBuilder added for the OperationAttempts, removed
the many overloads used for CommandResult, and updated all creation of the
CommandResult to use the builder.

See also #1518 to continue this
@amorton amorton mentioned this issue Oct 9, 2024
Merged
4 tasks
@amorton amorton self-assigned this Oct 10, 2024
This was referenced Oct 10, 2024
Closed
Merged
@amorton amorton removed their assignment Nov 5, 2024
@amorton amorton added the Code Refactor Refactoring to change structure (medium to large) label Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Code Refactor Refactoring to change structure (medium to large)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@amorton