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.

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

Move some namespaces on Exceptions #768

Merged
merged 8 commits into from
Apr 17, 2024

Conversation

loic425
Copy link
Member

@loic425 loic425 commented Nov 2, 2023

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Related tickets
License MIT

@loic425 loic425 requested a review from a team as a code owner November 2, 2023 13:29
src/Component/src/Exception/UnexpectedTypeException.php Outdated Show resolved Hide resolved

namespace Sylius\Resource\Exception;

final class VariantWithNoOptionsValuesException extends \Exception
Copy link
Member

Choose a reason for hiding this comment

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

Why this exception is necessary in sylius/resource, which doesn't know anything about variants?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know bro... should be on Product package I think

Copy link
Member Author

Choose a reason for hiding this comment

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

Issue created #867

src/Component/src/Exception/DeleteHandlingException.php Outdated Show resolved Hide resolved
src/Component/src/Exception/DeleteHandlingException.php Outdated Show resolved Hide resolved
@loic425 loic425 force-pushed the move-component-namespace/exception branch from 522bca7 to faf5da4 Compare March 25, 2024 08:26
@loic425
Copy link
Member Author

loic425 commented Mar 25, 2024

@diimpp I've just rebased this PR to solve conflicts but I still have to work on your suggestions.

@loic425 loic425 force-pushed the move-component-namespace/exception branch from 660d7dd to a70297e Compare April 9, 2024 15:50
@loic425
Copy link
Member Author

loic425 commented Apr 9, 2024

@diimpp @lchrusciel I've fixed conflicts again on this one.
We need to create issues about last comments.

@@ -22,6 +24,16 @@ function it_extends_an_exception(): void
$this->shouldHaveType(\Exception::class);
}

function it_is_a_runtime_exception(): void
Copy link
Member

@diimpp diimpp Apr 11, 2024

Choose a reason for hiding this comment

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

Should it be aliased instead and those new checks moved to new target namespace?

image

Copy link
Member

Choose a reason for hiding this comment

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

There are more specs, that needs to be aliased and content moved to new target namespace.

image

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right 👍 done

src/Component/src/Exception/UnexpectedTypeException.php Outdated Show resolved Hide resolved
@lchrusciel lchrusciel merged commit 643c575 into Sylius:1.11 Apr 17, 2024
28 checks passed
@lchrusciel
Copy link
Member

Thank you, @loic425!

@loic425 loic425 deleted the move-component-namespace/exception branch April 17, 2024 12:27
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.

None yet

3 participants