Skip to content
This repository has been archived by the owner on Nov 26, 2017. It is now read-only.

Use JApplicationBase as the foundation for JApplication as well. #847

Merged
merged 1 commit into from Feb 10, 2012

Conversation

realityking
Copy link
Contributor

The pull request partly reverts #843.

Instead of just having JApplicationCli and JApplicationWeb share a common base class this makes JApplication also extend JApplicationBase. To limit the amount of API change I only left those methods/variables in JApplicationBase that were shared by all 3 classes.

I think this would be a good first step in consolidating the new and old application classes.

@joomla-jenkins
Copy link

Unit testing complete. There were 0 failures and 0 errors from 1994 tests and 11169 assertions.
Checkstyle analysis reported 165 warnings and 0 errors.

@robschley
Copy link
Contributor

I think if we're going to make JApplication extend JApplicationBase, we should add the things that are missing, not remove them from JApplicationBase. You've removed some of the most important aspects of JApplicationBase like a standard place for input, events, and configuration. These should be added to JApplication if you want to work with all three apps in the same way.

-1

@robschley
Copy link
Contributor

I was mistaken about the input and events stuff being removed. Only the configuration stuff was removed. My apologies.

@realityking
Copy link
Contributor Author

As for the configuration, I don't see a way to do that in a backwards compatible fashion. JApplication uses the set() and get() methods from JObject which serve a completely different purpose. We can't change that without deprecating them first.

For the charset stuff I propose we just add them as abstract methods and leave the implementation to the subclasses, as far as I can tell they all need a slightly different implementation anyways.

Also after talking to Rob I added the loadDispatcher() method back to JApplicationBase and changed JApplication to use it.

@joomla-jenkins
Copy link

Build triggered by changes to the head.

Unit testing complete. There were 0 failures and 0 errors from 1994 tests and 11169 assertions.
Checkstyle analysis reported 165 warnings and 0 errors.

robschley added a commit that referenced this pull request Feb 10, 2012
Use JApplicationBase as the foundation for JApplication as well.
@robschley robschley merged commit cde4a88 into joomla:staging Feb 10, 2012
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants