-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[5.0] Update deleted files and folders to changes from #40293 #40298
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
[5.0] Update deleted files and folders to changes from #40293 #40298
Conversation
|
Is our script not able to delete directories recursively? |
@HLeithner The folder library's delete method does that, so yes. But in all 4.x versions and also in 3.10 we had it like that for whatever reason, so I continue with that. Maybe @wilsonge remembers why it was kept in this way. |
|
@HLeithner Maybe the reason was that it is harder to review and harder to write a tool which does it in that way, only deleting the top folder and not what's inside when that can be done. |
Haven't the foggiest :) |
|
pretty sure to reduce it folders only |
@HLeithner Sorry, I don't get it. What I know is that we have a script "build/deleted_file_check.php" which is used for checking deleted files and folders, and this lists them all, not only the topmost parent folder. |
| '/libraries/vendor/symfony/polyfill-php72/bootstrap.php', | ||
| '/libraries/vendor/symfony/polyfill-php72/LICENSE', | ||
| '/libraries/vendor/symfony/polyfill-php72/Php72.php', | ||
| '/libraries/vendor/symfony/polyfill-php73/bootstrap.php', | ||
| '/libraries/vendor/symfony/polyfill-php73/LICENSE', | ||
| '/libraries/vendor/symfony/polyfill-php73/Php73.php', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can remove this because it gets deleted in the folders array anyway. you also don't need to add subfolders to the array.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have generated the lists with a tool all the time up to now in the same way, and before I did that, it was done by others in that way. I will not change that now with this PR here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For example if you have some files to be deleted, and a few versions later then the folder can be deleted, the tool needs to tell which files and folders can be removed from the list. Currently it doesn't and so that would have to be done manually.
|
then why do we discuss it? waste of time... thanks anyway |
@HLeithner We can discuss that and I can work on improving the tool so it tells which files and folders can be removed from the list because due to later changes the complete folder can be removed. But it should not stop the current workflow of keeping the lists up to date. I agree that J5 would be a good point where we could change it. But as said, it requires some work. |
|
@HLeithner P.S.: A different approach was once discussed in this issue: #34298 . It would be to leave the lists like they are but delete the folders only if they are empty. |
Pull Request for Issue # .
Summary of Changes
This pull request (PR) updates the lists of files and folders to be deleted on update in file "administrator/components/com_admin/script.php" to recent changes in the 5.0-dev branch.
Currently there are only changes caused by PR #40293 for removing the "symfony/polyfill-php72" dependency.
Testing Instructions
Code review.
Or if you want to make a real test, update the last 4.4 nightly build to the last 5.0 nightly build to get the actual result, and update the last 4.4 nightly build to the update package built by Drone for this PR to get the expected result.
Actual result BEFORE applying this Pull Request
The files and folders of the "symfony/polyfill-php72" dependency are still present after updating from the last 4.4 nightly build.
Expected result AFTER applying this Pull Request
The files and folders of the "symfony/polyfill-php72" dependency have been deleted after updating from the last 4.4 nightly build, like it is done already without this PR here with the symfony polyfills for the other PHP versions.
Link to documentations
Please select:
No documentation changes for docs.joomla.org needed
No documentation changes for manual.joomla.org needed