? Pending

User tests: Successful: Unsuccessful:

avatar PhilETaylor
PhilETaylor
15 Aug 2021

Core review

This folder is never used. Therefore it doesnt need to exist.

avatar PhilETaylor PhilETaylor - open - 15 Aug 2021
avatar PhilETaylor PhilETaylor - change - 15 Aug 2021
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 15 Aug 2021
Category Repository Installation
avatar brianteeman brianteeman - test_item - 15 Aug 2021 - Tested unsuccessfully
avatar brianteeman
brianteeman - comment - 15 Aug 2021

I have tested this item ? unsuccessfully on 67617a9

doesnt remove the folder


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35140.

avatar brianteeman brianteeman - test_item - 15 Aug 2021 - Tested unsuccessfully
avatar PhilETaylor
PhilETaylor - comment - 15 Aug 2021

doesnt remove the folder

I never said it removed the folder. This PR doesnt remove the folder on sites that already have the folder in their installation folder.

This PR meets its aims as evidenced by the missing folder in the correct branch. https://github.com/PhilETaylor/joomla-cms/tree/removesessionsfolder/installation

There is no need to "clean up" existing folders like when we deleted folders in Joomla CMS elsewhere, because this is in the installation folder - and therefore is deleted on installation anyway and should not exist on a live site!

avatar richard67
richard67 - comment - 15 Aug 2021

The installation folder is indeed not handled by the files and folders deletion by purpose: https://github.com/joomla/joomla-cms/blob/4.0-dev/build/deleted_file_check.php#L63-L75

avatar PhilETaylor
PhilETaylor - comment - 15 Aug 2021

The installation folder is indeed not handled by the files and folders deletion by purpose:

Therefore this PR is correct and marking it with a an unsuccessful test is incorrect.

avatar richard67
richard67 - comment - 15 Aug 2021

The installation folder is indeed not handled by the files and folders deletion by purpose:

Therefore this PR is correct and marking it with a an unsuccessful test is incorrect.

@PhilETaylor I agree. @brianteeman could you revert your unsuccessful test? Thanks in advance.

avatar wilsonge
wilsonge - comment - 15 Aug 2021

This formerly was hardcoded here 14c78cf#diff-b000eaf18097ff802a6ad7bd96bd84543e8e232b2ded62dec482d0641d552defR56 . I think we've removed it when we moved here #18896 and we just use php temp directory now?

avatar PhilETaylor
PhilETaylor - comment - 15 Aug 2021

I think we've removed it when we moved here #18896 and we just use php temp directory now?

Further supporting the aims of this PR as being perfectly valid

avatar richard67
richard67 - comment - 15 Aug 2021

This formerly was hardcoded here 14c78cf#diff-b000eaf18097ff802a6ad7bd96bd84543e8e232b2ded62dec482d0641d552defR56 . I think we've removed it when we moved here #18896 and we just use php temp directory now?

Yes, seems so.

avatar richard67 richard67 - test_item - 15 Aug 2021 - Tested successfully
avatar richard67
richard67 - comment - 15 Aug 2021

I have tested this item successfully on 67617a9

The folder seems not to be used, at least I could not see it being accessed anyhow when making a new installation of J4.

When making a new installation where the folder has been removed before, the installation works well and there is no PHP error or warning logged.

Since the folder is below the installation folder, there is not need to care for removal on update.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35140.

avatar wilsonge wilsonge - change - 16 Aug 2021
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2021-08-16 20:15:37
Closed_By wilsonge
Labels Added: ?
avatar wilsonge wilsonge - close - 16 Aug 2021
avatar wilsonge wilsonge - merge - 16 Aug 2021
avatar wilsonge
wilsonge - comment - 16 Aug 2021

Thanks!

Add a Comment

Login with GitHub to post a comment