New Adapter: Aceex #1979
New Adapter: Aceex #1979
Conversation
|
Here is docs PR: prebid/prebid.github.io#3232 |
adapters/aceex/aceex.go
Outdated
| "github.com/prebid/prebid-server/openrtb_ext" | ||
| ) | ||
|
|
||
| type AceexAdapter struct { |
There was a problem hiding this comment.
We don't need to export this type. Can we rename to simply adapter?
adapters/aceex/aceex.go
Outdated
|
|
||
| func (a *AceexAdapter) checkResponseStatusCodes(response *adapters.ResponseData) error { | ||
| if response.StatusCode == http.StatusNoContent { | ||
| return nil |
There was a problem hiding this comment.
This exact check is found in line 146, which makes the true branch of this if statement to be unreachable. Can we remove either one or the other? It probably makes sense to remove the one in line 146 and leave this one but I'm good either way
| } | ||
|
|
||
| func getMediaTypeForImp(impId string, imps []openrtb2.Imp) openrtb_ext.BidType { | ||
| mediaType := openrtb_ext.BidTypeBanner |
There was a problem hiding this comment.
The current logic makes getMediaTypeForImp to return openrtb_ext.BidTypeBanner even if we didn't find a matching imp.ID in line 184. Other adapters throw an error if the imp.ID didn't find a match. Does that make sense in the context of your adapter? This is not a requirement, I'm ok either way.
There was a problem hiding this comment.
I think it's ok.
There was a problem hiding this comment.
@supportAceex thank you for adressing our feedback. Your code looks pretty good but test case adapters/aceex/aceextest/supplemental/status-code-no-content.json began failing in this last commit. Can you please correct before we approve?
|
Thank you! Fixed! |
* init aceex prebid-server adapter * remove gvlVendorID from config * fix review * fix unit tests
* init aceex prebid-server adapter * remove gvlVendorID from config * fix review * fix unit tests
No description provided.