Skip to content

[4.0] Remove unused model class#30600

Merged
wilsonge merged 1 commit intojoomla:4.0-devfrom
SharkyKZ:j4/php8/cmsmodel
Sep 15, 2020
Merged

[4.0] Remove unused model class#30600
wilsonge merged 1 commit intojoomla:4.0-devfrom
SharkyKZ:j4/php8/cmsmodel

Conversation

@SharkyKZ
Copy link
Copy Markdown
Contributor

@SharkyKZ SharkyKZ commented Sep 9, 2020

Pull Request for Issue # .

Summary of Changes

Removes unused Joomla\Component\Config\Site\Model\CmsModel class which now also errors on PHP 8 due to method signature mismatch.

Testing Instructions

Code review?

Documentation Changes Required

IDK.

@chmst
Copy link
Copy Markdown
Contributor

chmst commented Sep 9, 2020

Is it possible that 3rd party extension use this model?

@SharkyKZ
Copy link
Copy Markdown
Contributor Author

SharkyKZ commented Sep 9, 2020

It's possible. But to maintain PHP 8 compatibility we'd have to either update the method signatures or break class inheritance again, both of which are B/C breaks for anyone extending the class. So I think it's more painless to remove it and have developers extend Joomla\CMS\MVC\Model\BaseDatabaseModel instead. We have no way of testing if the model's code works in core anyways.

@ceford
Copy link
Copy Markdown
Contributor

ceford commented Sep 10, 2020

I have tested this item ✅ successfully on c27d011

I looked for CmsModel in php files throughout the code and found only one instance in CmsModel.php. I think this is a successful test.


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

@ceford
Copy link
Copy Markdown
Contributor

ceford commented Sep 10, 2020

Belay that last test - after applying the patch the class is still in the code.


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

@chmst
Copy link
Copy Markdown
Contributor

chmst commented Sep 10, 2020

I have tested this item ✅ successfully on c27d011


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

@ghost
Copy link
Copy Markdown

ghost commented Sep 11, 2020

RTC?

@richard67
Copy link
Copy Markdown
Member

@wilsonge Does anything speak against RTC with respect to the comment above?

@wilsonge
Copy link
Copy Markdown
Contributor

No we should be good as it's part of the component. Our B/C promise has never extended into components

@wilsonge wilsonge merged commit 979ace5 into joomla:4.0-dev Sep 15, 2020
@wilsonge
Copy link
Copy Markdown
Contributor

Thanks!

@wilsonge wilsonge added this to the Joomla 4.0 milestone Sep 15, 2020
sakiss pushed a commit to sakiss/joomla-cms that referenced this pull request Oct 16, 2020
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