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

Fix bug in countMenuChildren #792

Merged
merged 3 commits into from
Jan 24, 2012
Merged

Fix bug in countMenuChildren #792

merged 3 commits into from
Jan 24, 2012

Conversation

katalystsol
Copy link
Contributor

$query was generating an undefined variable error notice and then a fatal error notice because it is not defined previously in that method. After changing that on my site, which I had just upgraded to 2.5.0, it started working again.

…enerating an undefined variable error notice and then a fatal error notice because it is not defined previously in that method. After changing that on my site, which I had just upgraded to 2.5.0, it started working again.
@@ -493,11 +493,11 @@ public function countMenuChildren()
$active = $menu->getActive();
if ($active)
{
$query->getQuery(true);
Copy link
Contributor

Choose a reason for hiding this comment

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

This line should read $query = $dbo->getQuery(true);. The next 4 lines shouldn't need to be changed afterwards.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I just tested that on my site and it works with only changing the one line. Do I need to change this pull request then?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. Just update the file again and push the change and the pull request
will automatically update.

On 1/24/12 6:37 PM, "Don Cranford"
<reply+p-380497-462b1bc4a0c192e2c9834bb809e9c307f2312f33-368545@reply.githu
b.com> wrote:

@@ -493,11 +493,11 @@ public function countMenuChildren()
$active = $menu->getActive();
if ($active)
{

  •            $query->getQuery(true);
    

Thanks. I just tested that on my site and it works with only changing the
one line. Do I need to change this pull request then?


Reply to this email directly or view it on GitHub:
https://github.com/joomla/joomla-platform/pull/792/files#r380497

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the file. Thanks for your help.

@joomla-jenkins
Copy link

Test log missing. Tests failed to execute.
Checkstyle analysis reported 165 warnings and 1 errors.

@katalystsol
Copy link
Contributor Author

@joomla-jenkins ... I'm not sure what you're referring to. If I did something wrong, let me know. I'd like to learn how to work with this... The docs.joomla didn't say anything about a test log.

@mbabker
Copy link
Contributor

mbabker commented Jan 24, 2012

That is an automated test account to run the unit tests and code style rules. The unit tests most likely failed because of the first change you made. It looks like the tests will still fail as you've forgotten the $ on the $query = $dbo->getQuery(true); line.

@katalystsol
Copy link
Contributor Author

Ok. Thanks. I've corrected the line. I'm guessing the 165 warnings are from the rest of the file?

@mbabker
Copy link
Contributor

mbabker commented Jan 24, 2012

The full repository. All warnings that are being worked on.

On 1/24/12 8:55 PM, "Don Cranford"
<reply+i-2953661-0b060b7a03a0026bdc57d3cbe398a0ef3e45461f-368545@reply.gith
ub.com> wrote:

Ok. Thanks. I've corrected the line. I'm guessing the 165 warnings are
from the rest of the file?


Reply to this email directly or view it on GitHub:
#792 (comment)

@joomla-jenkins
Copy link

Build triggered by changes to the head.

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

eddieajau added a commit that referenced this pull request Jan 24, 2012
Fix bug in countMenuChildren
@eddieajau eddieajau merged commit 691d039 into joomla:staging Jan 24, 2012
@eddieajau
Copy link
Contributor

Good catch. Merged.

@imarklee
Copy link

Hey, this patch doesn't seem to be included in 2.5.6 .

@realityking
Copy link
Contributor

This is fixed in Joomla 3.0 alpha 1. If you need the fix in an older version please file a bug on the CMS tracker and link to this pull request.

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.

6 participants