Skip to content

Remove BlueprintBuilder as a possible runner input type #98

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.

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

Merged
merged 1 commit into from
Apr 4, 2024

Conversation

reimic
Copy link
Collaborator

@reimic reimic commented Apr 4, 2024

What does this PR do?

  • removes the possibility of parsing a Blueprint from a BlueprintBuilder

What problem does it fix?

  • the BlueprintParser's method parse could accept a BlueprintBuilder object as a potential input which could lead to errors

As context:

  • the only difference between passing a Blueprint and a BlueprintBuilder was that the method toBlueprint() was called for the builder, which has little utility
  • calling the method toBlueprint() on a specific instance of the BlueprintBuilder always returns the same instance of a Blueprint
  • this leads to an issue where someone:
    • creates a builder,
    • sets some of the variables,
    • runs the code with success,
    • sets some more variables,
    • runs it expecting another blueprint being generated and receives an error.
  • running the same Blueprint twice greatly confuses the program.

In conclusion:

  • the user expected the builder to generate another instance of a Blueprint, but it just set some values to the first instance. The runner extracted the first instance instead of creating a new one.

The explicit source of this particular error will be fixed in another PR, but as an example of what could go wrong:
InvalidArgumentException: Progress cannot go backwards (tried updating to 3.0105460023897 when it already was 3.0157410774586)

How to test if it works?

  • this PR only removes an execution path from the parser

@reimic reimic added the bug Something isn't working label Apr 4, 2024
@reimic reimic requested a review from adamziel April 4, 2024 15:00
@adamziel adamziel merged commit 87afea1 into WordPress:trunk Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants