Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[4.0]Remove manifest files and use existing XML info #16115

Closed
wants to merge 1 commit into from
Closed

[4.0]Remove manifest files and use existing XML info #16115

wants to merge 1 commit into from

Conversation

wilsonge
Copy link
Contributor

Summary of Changes

This is the first (in hopefully a series) of a cleanup of the Installer classes in Joomla 4, with the aim to make extension installs much more consistent and the classes more manageable.

Testing Instructions

Ensure installing libraries and packages is unchanged from the current state in 4.x

Documentation Changes Required

Classes removed.

Additional Notes

If this works after testing then of course the relevant deprecations must be made in 3.x but I'm not 100% confident of these changes (it feels far too easy) so I want some tests from others first.

@wilsonge wilsonge force-pushed the remove-manifest-v4 branch 2 times, most recently from dd71683 to b556eca Compare May 19, 2017 10:34
@wilsonge wilsonge requested a review from mbabker May 19, 2017 10:34
@mbabker mbabker added this to Testing/Review in [4.0] General May 19, 2017
@brianteeman brianteeman changed the title Remove manifest files and use existing XML info [4.0]Remove manifest files and use existing XML info May 19, 2017
@brianteeman
Copy link
Contributor

updated the title to show it is for J4

@laoneo
Copy link
Member

laoneo commented May 19, 2017

I guess you need to first merge #15884 into the 4.0-dev branch, otherwise you will probably get merge conflicts.

@wilsonge
Copy link
Contributor Author

wilsonge commented May 19, 2017 •
edited

For sure. As I said I just want testing because I can't believe I can delete this much code for adding an extra 5 lines into 2 files......

Note there are unit test fails unrelated to this PR because of an issue in the upstream framework application library - this is a general issue for the J4 branch - not something related to this PR

@wilsonge
Copy link
Contributor Author

Rebased with the #15884 in 4.0 - can i please have some testing :)

@C-Lodder
Copy link
Member

C-Lodder commented May 22, 2017 •
edited

Tried installing extensions from Regular Labs and Akeeba, both of which return an alert, Internal Error and also tried with a custom package.

Same thing happes on 4.0-dev

Ajax repsonse is throwing this error:

Call to undefined method Joomla\CMS\Installer\Adapter\ModuleAdapter::get()

@wilsonge
Copy link
Contributor Author

@C-Lodder your test hit a bug fixed by Michael with #16300 - now I've rebased this it should work :)

@brianteeman brianteeman modified the milestone: Joomla 4.0 Jun 8, 2017
@brianteeman
Copy link
Contributor

@wilsonge can you resolve conflicts please so this can be tested

@wilsonge wilsonge closed this Jul 24, 2017
@wilsonge wilsonge deleted the remove-manifest-v4 branch July 24, 2017 13:30
@wilsonge wilsonge removed this from the Joomla 4.0 milestone Jul 24, 2017
@wilsonge
Copy link
Contributor Author

Forgot this was still open - me and Michael found a problem which meant (as is) this can't work for packages. I'll sit down and try again another day

@brianteeman
Copy link
Contributor

ok thanks

@brianteeman
Copy link
Contributor

Can you check your other open issues - maybe the same is true for some of them as well https://github.com/joomla/joomla-cms/pulls/wilsonge

@zero-24 zero-24 removed this from Testing/Review in [4.0] General Jul 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbabker mbabker mbabker approved these changes

Assignees
No one assigned
Labels
Unit/System Tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@wilsonge @brianteeman @laoneo @C-Lodder @mbabker @joomla-cms-bot