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

Implementing #697 - Custom CPM bucket sizes and precision #742

Merged
merged 2 commits into from
Oct 25, 2016

Conversation

mkendall07
Copy link
Member

Type of change

  • Feature
  • Refactor

Description of change

Implements #697 - see ticket for details.

Other information

@prebid/core team for review.

Note: fixtures/cpmInputsOutputs.json was generated from the old bidmanager.getPriceBucketString function, to ensure no breaking changes during refactor.

resolve conflict

Fix typo.

Added `custom` price on bid object and set it automatically when using a custom config.

Docs

Remove trailing comma

Remove the need for `bucket.cap`. Automatically finds the highest `max` value to use for the cap.
@mkendall07
Copy link
Member Author

@matthewlane
Rebased and squashed now. Thanks

Copy link
Collaborator

@matthewlane matthewlane left a comment

Choose a reason for hiding this comment

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

Tested with custom implementation of 'dense' mode and standard dense mode, both work and price buckets match as expected. Refactor and new functionality LGTM

};

function getPriceBucketString(cpm, customConfig) {
var cpmFloat = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might want a let here for consistency

return (Math.floor(cpm * bucketSize) / bucketSize).toFixed(precision);
}

export {getPriceBucketString, isValidePriceConfig };
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nitpick: inconsistent spacing inside braces

@mkendall07 mkendall07 merged commit 8fed3d7 into master Oct 25, 2016
@mkendall07 mkendall07 deleted the feature/custom_price_buckets_clean branch January 10, 2017 15:10
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.

2 participants