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

Add analytics adapter by Sigmoid #2316

Merged
merged 14 commits into from
Apr 17, 2018
Merged

Conversation

sigmoidanalytics
Copy link
Contributor

@sigmoidanalytics sigmoidanalytics commented Mar 24, 2018

Type of change

  • New analytics adapter

Description of change

This is a new analytics adapter for Prebid by Sigmoid. We are an advanced analytical solutions company.
https://www.sigmoid.com/

@mkendall07
Copy link
Member

This pull request introduces 1 alert when merging 16c9063 into 8719aef - view on lgtm.com

new alerts:

  • 1 for Superfluous trailing arguments

Comment posted by lgtm.com

@sigmoidanalytics
Copy link
Contributor Author

@mkendall07 I have resolved all the errors and added markdown file. Can we move forward with this?

eventStack.events = [];
}

// function flushBidWon() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove dead code

function buildBidWon(eventType, args) {
bidWon.options = initOptions;
if (checkAdUnitConfig()) {
if (initOptions.adUnits.includes(args.adUnitCode)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use includes by importing it from core-js
import includes from 'core-js/library/fn/array/includes';
Usage includes(<array>, <item>

includes doesn’t work in older versions of IE. The imported version is polyfilled to work with ie, rather than Babel doing any kind of transforms that might end up not working in target browsers

// attach event listener
if (err) {
alert('Error retrieving credentials.');
console.error(err);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use utils.logError

AWS.config.credentials.get(function(err) {
// attach event listener
if (err) {
alert('Error retrieving credentials.');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove this alert

const url = 'https://kinesis.us-east-1.amazonaws.com/';
const analyticsType = 'endpoint';

let auctionInitConst = CONSTANTS.EVENTS.AUCTION_INIT;
Copy link
Collaborator

Choose a reason for hiding this comment

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

These should be const

let adaptermanager = require('src/adaptermanager');
let constants = require('src/constants.json');

describe('sigmoid Prebid Analytic', function () {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your coverage is 70%. Please increase that to atleast 80%
To test and view use gulp test-coverage and gulp view-coverage

@jaiminpanchal27
Copy link
Collaborator

@sigmoidanalytics Any update ?

@jaiminpanchal27
Copy link
Collaborator

@sigmoidanalytics Travis failed due to lint error. Please check

@jaiminpanchal27 jaiminpanchal27 merged commit 1b29d1f into prebid:master Apr 17, 2018
ArmandChoy pushed a commit to RockYou-Ads/Prebid.js that referenced this pull request Apr 18, 2018
* 'master' of https://github.com/prebid/Prebid.js: (211 commits)
  Increment Pre Version
  Prebid 1.8.0 Release
  Make eslint aware of the custom import paths (prebid#2292)
  send travis-ci notifications to slack (prebid#2404)
  send appnexus usePaymentRule info to prebid-server ortb request (prebid#2351)
  convert AN bid params to underscore formatting for pbs (prebid#2385)
  EbdrAdapter add usersync  (prebid#2407)
  Add outstream renderer to Beachfront adapter (prebid#2403)
  Add analytics adapter by Sigmoid (prebid#2316)
  deprecate loadScript and add loadExternalScript (prebid#2391)
  Removed the ability for to override any standard query parameters (prebid#2402)
  Unit test failures (prebid#2405)
  Add Unruly Bid Adapter (prebid#2326)
  Added VIS.X Bidder Adapter (prebid#2359)
  Smart: Add prebid version in the data payload (prebid#2394)
  add support for video bids to use an impression tracking URL  (prebid#2365)
  Create rtbdemandAdkBidAdapter_spec.js (prebid#2352)
  Widespace adapter (prebid#2283)
  Add: Vuble Analytics Adapter (prebid#2331)
  fixes prebid#2353 - not appending hb_uuid and hb_cache_id (prebid#2363)
  ...

# Conflicts:
#	test/spec/modules/rockyouBidAdapter_spec.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants