Skip to content

[5.3] Clean Up File::upload method arguments#45244

Merged
laoneo merged 1 commit intojoomla:5.3-devfrom
joomdonation:code_clean_up
Apr 17, 2025
Merged

[5.3] Clean Up File::upload method arguments#45244
laoneo merged 1 commit intojoomla:5.3-devfrom
joomdonation:code_clean_up

Conversation

@joomdonation
Copy link
Contributor

Pull Request for Issue # .

Summary of Changes

A minor bug fix/ clean up to File::upload() method arguments after we migrated from CMS FileSystem to Framework Filesystem package. Basically, unlike the CMS FileSystem package, the File::upload() method in Framework Filesystem package only has 3 parameters, see https://github.com/joomla-framework/filesystem/blob/3.x-dev/src/File.php#L285

So passing the fourth argument in File::upload call is not correct (PHP currently ignores the extra parameter, but it should be fixed anyway). Also, the third argument has same value with default value in method definition, so it could be omitted to make it consistent with other File::upload calls in our code.

Testing Instructions

  • Use Joomla 5.3 nightly build
  • Apply patch
  • Try to install an extension

Or code review should be enough, too.

Actual result BEFORE applying this Pull Request

Works

Expected result AFTER applying this Pull Request

Works

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

@exlemor
Copy link

exlemor commented Mar 29, 2025

I have tested this item ✅ successfully on 9c260f5

I have tested this successfully- installed and uninstalled a few Components (small to complex), as well as modules, and plugins.

Thanks @joomdonation!


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

@krishnagandhicode
Copy link
Contributor

I have tested this item ✅ successfully on 9c260f5

Thanks @joomdonation!


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

@alikon
Copy link
Contributor

alikon commented Mar 29, 2025

RTC


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

@joomla-cms-bot joomla-cms-bot added RTC This Pull Request is Ready To Commit and removed PR-5.3-dev labels Mar 29, 2025
@laoneo laoneo merged commit 76f835e into joomla:5.3-dev Apr 17, 2025
4 of 5 checks passed
@laoneo
Copy link
Member

laoneo commented Apr 17, 2025

Thanks!

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Apr 17, 2025
@laoneo laoneo added this to the Joomla! 5.3.1 milestone Apr 17, 2025
@joomdonation joomdonation deleted the code_clean_up branch April 17, 2025 16:02
richard67 pushed a commit to richard67/joomla-cms that referenced this pull request Apr 23, 2025
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.

6 participants