Skip to content

Conversation

@laoneo
Copy link
Member

@laoneo laoneo commented Aug 23, 2018

Pull Request for pr #21790.

Summary of Changes

As #21790 revealed, the manifest files in core are out of sync. Before I'm going to fix all of them I would like to get some feedback what all needs to be adapted.

Beside the folders I would also change the compatibility version and the version of the extension itself. Anything else?

<filename>controller.php</filename>
<folder>controllers</folder>
<folder>elements</folder>
<folder>Controller</folder>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing
filename content.xml

@brianteeman
Copy link
Contributor

other than the missing file it looks ok to me

would be good to get another check though

<folder>models</folder>
<folder>Model</folder>
<folder>Service</folder>
<folder>Table</folder>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if i'm not wrong there is no Table folder

@laoneo
Copy link
Member Author

laoneo commented Aug 24, 2018

Added also media folder.

@brianteeman
Copy link
Contributor

I am wondering if we even need these manifest files for core components and with the move of media files if they should be in the manifest either

@mbabker
Copy link
Contributor

mbabker commented Aug 28, 2018

Yes the extension manifests should be in place. Technically, we don't need them, but the core package shouldn't be taking liberties as it relates to extension structure that a distributed extension can't.

@brianteeman
Copy link
Contributor

And the media folder? The same?

@laoneo did you have to create this manually - if so I am happy to takeover so you can concentrate on developer stuff

Copy link
Contributor

@wilsonge wilsonge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need the media tag - we aren't shipping a media folder with the extension as it's all baked into core (especially now with the es6 stuff) - so I'd leave that as it is. Otherwise LGTM

@laoneo
Copy link
Member Author

laoneo commented Aug 29, 2018

For me it reflects the installed state of an extension which includes the media folder and com_content has a media folder. From my understanding an extension dev should be able to take it as example and the manifest with the media folder is part of it.

I can remove it if you really want but I think it is not a bad idea to leave it in.

@brianteeman yes I'm doing them by hand. If you want you can take over the task when we get this one merged. But I would wait about the agreement of the media folder.

@brianteeman
Copy link
Contributor

When you decide on media let me know and i can get this done

@wilsonge wilsonge merged commit bf36374 into joomla:4.0-dev Sep 10, 2018
@wilsonge wilsonge deleted the j4/manifest/content branch September 10, 2018 10:39
@wilsonge
Copy link
Contributor

Not convinced about the media tag - but not sceptical enough to block progress here

@wilsonge wilsonge added this to the Joomla 4.0 milestone Sep 10, 2018
brianteeman added a commit to brianteeman/joomla-cms that referenced this pull request Sep 12, 2018
Based on the work by @laoneo in joomla#21817

Code review only
wilsonge pushed a commit that referenced this pull request Sep 12, 2018
Based on the work by @laoneo in #21817

Code review only
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