Skip to content
This repository has been archived by the owner on Mar 1, 2022. It is now read-only.

Analyzer runner w/ push notifications - squashed #65

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

Conversation

pdziok
Copy link
Contributor

@pdziok pdziok commented Aug 21, 2015

No description provided.

$this->samples = ceil($secondsToAnalyze / $frequency);
}

public function analyze()
Copy link
Member

Choose a reason for hiding this comment

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

This method is duplicated between GraphAnalyzer and NumberAnalyzer - please refactor.

@domeq
Copy link
Member

domeq commented Aug 24, 2015

I'm trying to figure out from the code, but I'm not 100% sure: is this 100% compatible with dashoards WITHOUT access to VG Push Platform API? What I mean is: if I don't supply keys in my config, will whole functionality be disabled and dashboard will work same as before?

@pdziok
Copy link
Contributor Author

pdziok commented Aug 24, 2015

@domeq: to enable analyzer another cli process must be started - it does not affect standard rtm behaviour

@pdziok
Copy link
Contributor Author

pdziok commented Aug 24, 2015

should I also fix phpcs errors from different classes (outside the PR scope)?

@domeq
Copy link
Member

domeq commented Aug 24, 2015

No, these are on me :)

@domeq
Copy link
Member

domeq commented Aug 24, 2015

phpcs fixed in master (all but one)

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.

2 participants