[#40093] - [5.1] Extending Joomla\CMS\Filesystem\File from Joomla\Filesystem\File
- Closed
- 25 Feb 2024
- Medium
- Build: 5.0-dev
- # 40093
- Diff
- Hackwar:5.0-filesystem-file
User tests: Successful: Unsuccessful:
Summary of Changes
This PR extends the CMS File class from the framework Filesystem File class. This removes all duplicate code from the CMS class, using the framework methods where possible. This means that now this class throws exceptions and that all FTP remnants are removed.
Looking at the remaining code, we should consider moving some of this into the framework as well. We probably should deprecate canFlushFileCache() as well.
I've changed this a bit agressively, no remaining method stubs with deprecation notices, no hints at the removed FTP code, but I hope that this is still okay.
Testing Instructions
Everything should work like before... I think this should rather be done by codereview.
Link to documentations
Please select:
-
Documentation link for docs.joomla.org:
-
No documentation changes for docs.joomla.org needed
-
Pull Request link for manual.joomla.org:
-
No documentation changes for manual.joomla.org needed
joomla-cms-bot
- | Category | ⇒ | Libraries |
| Status | New | ⇒ | Pending |
| Labels |
Added:
PR-5.0-dev
|
||
Is there some code which needs to stay in the CSM File class? From a quick glance most of it can be moved to the framework package. Then I would even go a step further and deprecate the whole class.
I'm working on a deprecation PR for the whole filesystem package in the CMS library for 4.4. Will take me some time. Most likely I will have to re-do these PRs here then again.
Yes, I know. But we have to do it at some point and 4.4 is the point.
The comments in your PR are also something that we have to fix in the framework package then.
I did some further cleanup and copied code over to the framework Filesystem package. This means that for this to be able to be merged, joomla-framework/filesystem#54 and joomla-framework/filesystem#55 need to be merged first. This leaves the upload() method as the last one which we need to replace to really completely remove this class in 6.0.
| Labels |
Added:
?
b/c break
|
||
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2023-04-18 12:06:41 |
| Closed_By | ⇒ | Hackwar |
| Status | Closed | ⇒ | New |
| Closed_Date | 2023-04-18 12:06:41 | ⇒ | |
| Closed_By | Hackwar | ⇒ |
| Status | New | ⇒ | Pending |
This pull request has been automatically rebased to 5.1-dev.
| Labels |
Added:
Feature
Removed: ? |
||
| Labels |
Added:
PR-5.1-dev
|
||
| Title |
|
||||||
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2024-02-25 09:14:36 |
| Closed_By | ⇒ | Hackwar |
I doubt this will ever be merged, so I'm closing this.
same as Folder, can't be done this way.
Also we don't use underscore for class names, in this case we use PascalCase