#2482783: File upload errors not set or shown correctly added an internal method to file.module, _file_save_upload_from_form() which helped ensure form errors we displayed during file uploads. The method was meant to be an internal fix in the file module pending a larger refactoring of file handling code into a service. The method was marked @internal and @deprecated, but should just have been marked @internal, to be removed in whichever minor release the refactoring occurs
Remove @deprecated and leave @internal and update the documentation
review
commit
none
Technically none. The method was already internal, so not api.
none
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | 3069020-3.drupal.Undeprecate-filesaveuploadfromform-and-keep-internal.patch | 1.3 KB | mikelutz |
Comments
Comment #2
alexpottThis one is super complex but it is not really deprecated at al. See #2482783: File upload errors not set or shown correctly. I think we should undeprecate it. It’s marked @internal anyways and we could move the docs that are in the deprecated to the internal bit.
The current deprecation tells you to use the managed file form element but that actually calls this code. If we wanted to deprecate this we'd have to move this code into \Drupal\file\Element\ManagedFile and make the places in core that currently use it use that form element.
Note there are long discussions on #2482783: File upload errors not set or shown correctly about adding it as deprecated so we probably need to revisit them and collect them here to make sure we don;t overlook anything.
Comment #3
mikelutzmarking @internal and @deprecated was first mentioned in #2482783-70: File upload errors not set or shown correctly, and only in the sense that it was to be a temporary solution and so it should be @internal and @deprecated, and this is before we really defined what @deprecated means.
@deprecated means the code is not called in core outside of tests, and the item will be removed in the next (or occasionally next next) major version, while @internal means the code IS used by core, but MAY be removed/changed in any minor version (perhaps with a CR). Either core uses the code outside of legacy tests, in which case it is not @deprecated, or core doesn’t use the code at all, which means it’s not @internal (or more specifically if it’s @internal and core doesn’t use the code, it should just be removed in the next minor, not marked @deprecated)
This code is used outside of tests intentionally, so it should be @internal, but not @deprecated. If/When the system is refactored, the @internal method can be removed in any minor release without being deprecated.
This patch removes the @deprecated tag and updated the @internal docblock comment.
+ * This function is internal, and may be removed in a minor version release.could arguably be considered redundant, but I think it makes it very clear.Comment #4
berdirMakes sense to me.
Comment #5
larowlanCommitted bf3295d and pushed to 8.8.x. Thanks!