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

[Maintenance] Add an exception for unavailable storages #862

Merged
merged 2 commits into from
Apr 19, 2024

Conversation

NoResponseMate
Copy link
Contributor

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

There's a bunch of issues open Sylius regarding cli/stateless actions resulting in fatals since session is not available when resolving something within a context.
Would be nice to have a unified way of knowing whether the storage medium can be accessed at all.

@NoResponseMate NoResponseMate added Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). DX Issues and PRs aimed at improving Developer eXperience. labels Apr 8, 2024
@NoResponseMate NoResponseMate requested a review from a team as a code owner April 8, 2024 10:55
diimpp
diimpp previously requested changes Apr 11, 2024
@@ -13,7 +13,9 @@

namespace Sylius\Bundle\ResourceBundle\Storage;

use Sylius\Component\Resource\Exception\StorageUnavailableException;
Copy link
Member

Choose a reason for hiding this comment

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

Since there is ongoing initiative to move namespaces in 1.11, new exception should go directly into target namespace.

Suggested change
use Sylius\Component\Resource\Exception\StorageUnavailableException;
use Sylius\Resource\Exception\StorageUnavailableException;

@@ -14,7 +14,9 @@
namespace spec\Sylius\Bundle\ResourceBundle\Storage;

use PhpSpec\ObjectBehavior;
use Sylius\Component\Resource\Exception\StorageUnavailableException;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
use Sylius\Component\Resource\Exception\StorageUnavailableException;
use Sylius\Resource\Exception\StorageUnavailableException;


declare(strict_types=1);

namespace Sylius\Component\Resource\Exception;
Copy link
Member

Choose a reason for hiding this comment

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

If for some reason legacy exception needs to be created, then it should become class alias, otherwise please move it to src/Component/src/Exception/StorageUnavailableException


namespace Sylius\Component\Resource\Exception;

class StorageUnavailableException extends \RuntimeException
Copy link
Member

Choose a reason for hiding this comment

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

Exceptions are modernized at new non-component namespace, please apply this change.

Suggested change
class StorageUnavailableException extends \RuntimeException
use Sylius\Resource\Exception\RuntimeException;
class StorageUnavailableException extends RuntimeException

image

@@ -13,24 +13,39 @@

namespace Sylius\Resource\Storage;

use Sylius\Component\Resource\Exception\StorageUnavailableException;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
use Sylius\Component\Resource\Exception\StorageUnavailableException;
use Sylius\Resource\Exception\StorageUnavailableException;

@NoResponseMate
Copy link
Contributor Author

@diimpp
Wanted to wait until #768 gets merged to avoid unnecessary conflicts.
Updated 👋

@lchrusciel lchrusciel dismissed diimpp’s stale review April 19, 2024 09:31

Review already applied

@lchrusciel lchrusciel merged commit 0c5efcf into Sylius:1.11 Apr 19, 2024
28 checks passed
@lchrusciel
Copy link
Member

Thank you, @NoResponseMate!

@NoResponseMate NoResponseMate deleted the add-storage-exception branch April 29, 2024 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DX Issues and PRs aimed at improving Developer eXperience. Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants