Skip to content
This repository has been archived by the owner on Aug 2, 2023. It is now read-only.

Resolve issues#13 #16

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Resolve issues#13 #16

wants to merge 1 commit into from

Conversation

itsazzad
Copy link
Contributor

@itsazzad itsazzad commented Oct 6, 2016

Resolve issues#13

Resolve issues#13
Copy link
Contributor

@mcelicalderon mcelicalderon left a comment

Choose a reason for hiding this comment

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

Please address changes so this PR can be merged.

Thanks!

@@ -137,7 +137,7 @@ private function _findOrCreateCallTrackingApp()
"friendlyName" => 'Call tracking app'
)
);
if ($existingApp) {
if (count($existingApp)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @itsazzad ! #13 Was already resolved in #14 . This change you are introducing shouldn't make any difference as an empty array will be evaluated as false just as much as a 0 that you get from counting said array. But, I would actually like to be more explicit here and replace that expression with

if (count($existingApp) > 0) {

If you wouldn't mind making that change, I'd be happy to merge your PR. And we have the exact same expression here that I think was also introduced by you.

Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants