Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature 69 nested aggregators #52

Merged
merged 18 commits into from
Dec 11, 2016
Merged

Feature 69 nested aggregators #52

merged 18 commits into from
Dec 11, 2016

Conversation

oesse
Copy link
Member

@oesse oesse commented Dec 11, 2016

No description provided.

@oesse oesse requested a review from fabian247 December 11, 2016 11:33
@coveralls
Copy link

Coverage Status

Coverage decreased (-1.2%) to 98.828% when pulling 8d1d9b0 on feature-69-nested-aggregators into 38a3ba9 on dev.

Copy link
Contributor

@jhuenges jhuenges left a comment

Choose a reason for hiding this comment

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

Sieht sehr schick aus :)

*/
class Max {
/**
* Construct a Max Aggregator using an array of aggregatiors.
Copy link
Contributor

Choose a reason for hiding this comment

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

aggregators

} else if (!aggregator.combine) {
throw new InvalidInputError(`Aggregator has no combine function`)
}
// if (typeof aggregator === 'string') {
Copy link
Contributor

Choose a reason for hiding this comment

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

Comments bitte entfernen

function getAggregator (aggregator) {
return typeof aggregator === 'string' ? new aggregators[aggregator]() : aggregator
}
// /**
Copy link
Contributor

Choose a reason for hiding this comment

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

Comments bitte entfernen

@@ -1,6 +1,7 @@
const buster = require('buster')
const scoreManager = require('../lib/score-manager')
const aggregator = require('../lib/score-aggregator')
// const aggregator = require('../lib/score-aggregator')
Copy link
Contributor

Choose a reason for hiding this comment

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

Comments bitte entfernen

buster.testCase('ScoreManager Integration', {
buster.testCase('ScoreManager Plugin Integration', {
setUp: function () {
// this.stubAstEval = this.stub()
Copy link
Contributor

Choose a reason for hiding this comment

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

Comments bitte entfernen

/**
* Return a WeightedMean Aggregator from an aggregator array. The parameters
* will be forwarded to WeightedMean.constructor().
*/
Copy link
Member

Choose a reason for hiding this comment

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

In Z.25 und 26 sollte wohl auch immer Max statt WeightedMead stehen.

@@ -260,7 +262,10 @@ class ScoreManager {

let successfulScores = {}
successfulKeys.forEach(key => { successfulScores[key] = scores[key] })
return this.aggregator.combine(successfulScores)

let ast = aggregatorConfigParser.parse(
Copy link
Member

@fabian247 fabian247 Dec 11, 2016

Choose a reason for hiding this comment

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

ast -> aggregatorSourceTree ?
Variable bitte umbenennen für besseres Verständnis

@oesse oesse assigned jhuenges and fabian247 and unassigned oesse Dec 11, 2016
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 99.574% when pulling 7778ea7 on feature-69-nested-aggregators into 38a3ba9 on dev.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling e7413d2 on feature-69-nested-aggregators into 38a3ba9 on dev.

@jhuenges jhuenges removed their assignment Dec 11, 2016
@fabian247 fabian247 merged commit 9790222 into dev Dec 11, 2016
@simonschwan simonschwan deleted the feature-69-nested-aggregators branch January 26, 2017 12:50
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.

None yet

4 participants