Skip to content

Conversation

@wilsonge
Copy link
Contributor

@wilsonge wilsonge commented Feb 26, 2024

Various minor reworkings to the TUF code as below

Summary of Changes

  • Removes the extra HTTP Factory - it serves no purpose. Whilst we don't implement PSR-18 yet it shows the type of object to be passed around which is a created HTTP client - not the factory for it: https://www.php-fig.org/psr/psr-18/
  • Rework constructor args - db is now required (it already existed in one of the two instantiating classes anyhow and we call Factory::getDbo() already in the other), makes application a required parameter (we were statically grabbing it in the class - which shouldn't be the case for new classes).
  • Fixes the constructor doc block

Testing Instructions

Check all TUF functionality from #42799 continues to work with no changes. New code is slightly easier to maintain and read.

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

@wilsonge wilsonge changed the title Move around some TUF constructor args [5.1] TUF Code Cleanup Feb 26, 2024
@wilsonge
Copy link
Contributor Author

@SniperSister would be good to understand why we're forcing curl to make the http requests rather than the any of the 3 options the server might support

Co-authored-by: Harald Leithner <[email protected]>
@alikon
Copy link
Contributor

alikon commented Mar 12, 2024

hopefully i've done it in a right way
but it works for me now

@alikon
Copy link
Contributor

alikon commented Mar 12, 2024

I have tested this item ✅ successfully on 0eda369


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

1 similar comment
@brianteeman
Copy link
Contributor

I have tested this item ✅ successfully on 0eda369


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

@brianteeman
Copy link
Contributor

Before this PR my test server without curl would fail to check for updates etc
With this PR there was no problem


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

@alikon
Copy link
Contributor

alikon commented Mar 12, 2024

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Mar 12, 2024
@richard67
Copy link
Member

@alikon Have you seen that PHPCS fails in Drone?

@alikon
Copy link
Contributor

alikon commented Mar 12, 2024

no, cause
sometimes drone failure are ..... 😄

@richard67
Copy link
Member

no, cause sometimes drone failure are ..... 😄

@alikon You can't check the drone log to see what fails?

@alikon
Copy link
Contributor

alikon commented Mar 12, 2024

not on my local

@richard67
Copy link
Member

not on my local

@alikon I mean on GitHub with the link beside the test result. The thing is that when PHPCS fails, no integration or system or unit tests are run, so we cannot see if something's wrong with them.

Anyway I've fixed it now.

@alikon
Copy link
Contributor

alikon commented Mar 12, 2024

@richard67 i mean sometimes you got drone failing for timoeout and... something else ..,etc
thanks for fixing it

@LadySolveig LadySolveig merged commit a79eeda into joomla:5.1-dev Mar 14, 2024
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 14, 2024
@LadySolveig
Copy link
Contributor

Thanks to all involved 🚀

@Quy Quy added this to the Joomla! 5.1.0 milestone Mar 14, 2024
@wilsonge wilsonge deleted the tuf/fix-stuff branch March 14, 2024 23:16
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.