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.

Sign up for GitHub

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

Jump to bottom

Add new Adapter- Beachfront Media #1062

Merged
merged 34 commits into from
Apr 18, 2017
Merged

Add new Adapter- Beachfront Media #1062

merged 34 commits into from
Apr 18, 2017

Conversation

Allen-37F
Copy link
Contributor

@Allen-37F Allen-37F commented Mar 21, 2017 •
edited
Loading

Type of change

  • Bugfix
  • Feature
  • New bidder adapter
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other

Description of change

  • test parameters for validating bids
{
  	bidder: 'beachfront',
           params: {
              bidfloor: .01,
              appId: "11bc5dd5-7421-4dd8-c926-40fa653bec76"
          	}
        	}
        ]
}

Other information

Allen-37F and others added 25 commits February 20, 2017 18:15
@Allen-37F
init
06ad614
@Allen-37F
Merge branch 'master' of https://github.com/Allen-37F/Prebid.js
494b984
@Allen-37F
refactoring and replacing hard-coded variables
a9d0c21
@Allen-37F
changed error handling if missing adid or bidfloor
2b9d2ee
@Allen-37F
created testing spec for beachfront adapter & began writing unit tests
5a8a22e
@Allen-37F
working on testing
a027614
@Allen-37F
still testing
d37a1f8
@Allen-37F
all tests passing but the last two. Almost there...
8797904
@Allen-37F
getting ready to change to new getmu response
494f428
@Allen-37F
adapter working, tests complete; removing console logs and setting up…
2ef8413 
… for prebid.js pull request
@Allen-37F
only the beachfront adapter plus prebidvid.html
79feb7d
@Allen-37F
merging with upstream master
e517f05
@Allen-37F
Added adapter for Beachfront and associated tests.
0990e13
@Allen-37F
updating adapter
179cf87
@Allen-37F
Merge commit '179cf87360d0a5e03ed6cdb28b608571c958d45b' into bfAdapter
e51e7cf 
# Conflicts:
#	src/adapters/beachfront.js
@Allen-37F
Ready for pull request with prebid.js
71e6e6f
@Allen-37F
Merge branch 'temp'
015510d 
* temp:
  Ready for pull request with prebid.js
@Allen-37F
Merge commit '0cea31cd294f380b3b7cf46dd7a4000316b71ac1'
da9de35 
* commit '0cea31cd294f380b3b7cf46dd7a4000316b71ac1':
  Fix Mantis tests in negative timezone (prebid#1049)
  Add deal id handling (prebid#1044)
  sanitize bidderRequest to rubicon adapter to ensure accountId is sent (prebid#1030)
  Bidfluence Adapter (prebid#1023)
@Allen-37F
Updated ‘dev’ parameter in test request
f650de8
@Allen-37F
Update params for pull request
985fcad
@Allen-37F
Pointing to prod endpoint in adapter and test
4a7668f
@Allen-37F
Stage adapter files for prebid pull request
3d3f90e
@Allen-37F
Retrying commit of adapter files for PR
c4263f2
@Allen-37F
Merge branch 'master' of https://github.com/prebid/Prebid.js into pre…
ce73cd8 
…bid_pr
@Allen-37F
last-minute
b5d9852
@aneuway2
Copy link
Contributor

Hey @Allen-37F are bidFloor and appId both required parameters for your adaptor? Might be good to submit a PR to have this documented :-)

https://github.com/prebid/prebid.github.io/tree/master/dev-docs/bidders

@Allen-37F
Copy link
Contributor Author

Yes, those are the two required params. Thanks for the tip - added a .md documenting that.

@Allen-37F Allen-37F changed the title Prebid pr Add new Adapter- Beachfront Media Mar 27, 2017
@snapwich
Copy link
Collaborator

Thanks for the test page comment. It should not be a part of the pull-request commits though, sorry if I was confusing.

@Allen-37F
Copy link
Contributor Author

Roger that - deleted index.html from the pull request

Allen-37F
Allen-37F commented Apr 12, 2017
View reviewed changes
Copy link
Contributor Author

@Allen-37F Allen-37F left a comment

Choose a reason for hiding this comment

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

I hope this did the trick!

@snapwich
Copy link
Collaborator

LGTM 👍

mkendall07
mkendall07 requested changes Apr 17, 2017
View reviewed changes
src/adapters/beachfront.js Outdated
var newBid = {};
newBid.price = parsed.bidPrice;
newBid.url = parsed.url;
newBid.bidId = utils.getUniqueIdentifierStr();
Copy link
Member

Choose a reason for hiding this comment

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

The bidId should be from the request bid - see https://github.com/prebid/Prebid.js/blob/master/src/adapters/appnexus.js#L200 for example. This is to sync the request/response.

@mkendall07
Copy link
Member

@Allen-37F please update the code for handling the bid.bidId and then we can mrege.
@snapwich not sure if the bug you referring to relates to the bid ID not matching or not.

@snapwich
Copy link
Collaborator

@mkendall07 I don't think it's related, but that's a good catch.

@mkendall07
Copy link
Member

LGTM.

@mkendall07 mkendall07 merged commit e532d09 into prebid:master Apr 18, 2017
@mkendall07 mkendall07 removed the ready label Apr 18, 2017
@mkendall07
Copy link
Member

This is merged into master. Please submit a PR to the docs repo to add a file for your adapter to the bidders directory so your adapter's params will appear on the bidders page. Thank you for contributing

@Allen-37F
Copy link
Contributor Author

Thanks, everyone! The PR to have Beachfront added to the bidders directory is #203, and it's ready.

outoftime added a commit to Genius/Prebid.js that referenced this pull request Apr 19, 2017
@outoftime
Merge branch 'master' of https://github.com/prebid/Prebid.js into pre…
b930704 
…built

* 'master' of https://github.com/prebid/Prebid.js: (38 commits)
  Add optional domain parameter to AdButler adapter (prebid#1078)
  Send transactionID to Criteo Services (prebid#1113)
  Fix `buildMasterVideoTagFromAdserverTag()` not selecting winning bid (prebid#1106)
  Remove placement size selection and filtering (prebid#1107)
  revert `srcdoc` change (prebid#1130)
  Add new Adapter- Beachfront Media (prebid#1062)
  Fixes SpringServe adapter (prebid#1101)
  Update Widespace request param (prebid#1098)
  - New Adapter: Innity (prebid#1074)
  Update Roxot prebid analytic adapter (prebid#1034)
  Yarn Package Manager (prebid#1109)
  allow writing into current document if prebid is loaded inside an iframe (prebid#1066)
  Adapter bug fix (prebid#1096)
  fix typo
  added pr review process and governance model (prebid#1103)
  added support for sampling in ga and base adapter, fixed up some tests (prebid#1011)
  Add Inneractive adapter (prebid#1048)
  Add alias freewheel-ssp to stickyadstv bidder adapter  (prebid#1043)
  Add Facebook Audience Network adapter (prebid#1068)
  Add Atomx support (prebid#1056)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mkendall07 mkendall07 mkendall07 approved these changes

@snapwich snapwich snapwich approved these changes

Assignees

@snapwich snapwich

Labels
LGTM video
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@Allen-37F @aneuway2 @snapwich @mkendall07 @jaiminpanchal27 @mjacobsonny @ndhimehta