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

StorageInterface resolveStream nullable $fieldName #1373

Merged

Conversation

pauliuspetronis
Copy link
Contributor

No description provided.

Copy link
Collaborator

@garak garak left a comment

Choose a reason for hiding this comment

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

An explanation would be much appreciated

@pauliuspetronis
Copy link
Contributor Author

Other 2 functions (resolvePath and resolveUri) accept fieldName as nullable string. Why resolveStream function makes different? I think it like a mistake or misstype. If you have one "uploadable" field per entity - you have to pass field name every time only on this function.

* @param string|null $className The object's class. Mandatory if $obj can't be used to determine it
*
* @return resource|null The resolved resource or null if file not stored
*/
public function resolveStream(object|array $obj, string $fieldName, ?string $className = null);
public function resolveStream(object|array $obj, ?string $fieldName = null, ?string $className = null);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a BC break, as any changes in an interface.
But I understand that it's currently inconsistent, and it's not a big change.
Anyway, we need to annotate this change in the UPGRADE file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

UPGRADE.md Outdated
@@ -1,3 +1,7 @@
# Upgrading from v2.1.1 to v2.2.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn't seem right. I understand that you were misled by the other entry (which is also wrong), but specifying the patch version here doesn't make sense.
Please change it to "from 2.1 to 2.2". Please change the other one accordingly ("from 2.0 to 2.1").

UPGRADE.md Outdated
# Upgrading from v2.0.1 to v2.1.0
# Upgrading from v2.1 to v2.2

* The signature of `StorageInterface::resolveStream` method was changed. Nullable value of parameters `$fieldName`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

$fieldName is a single parameter, right?
Anyway, this sentence is not clear, please reword it as follows: "The $fieldName parameter is now nullable."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Sorry for my en.

@garak garak merged commit 4288239 into dustin10:master Mar 13, 2023
@pauliuspetronis pauliuspetronis deleted the storage-interface-nullable-fieldName branch March 14, 2023 17:17
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

2 participants