Release/4.2.3.0.0 #1
Conversation
ricealexanderb
requested review from
a team,
bwised,
daniel-barros,
kendallrogers and
CB-HaoxinLi
and removed request for
a team
| extension BidMachineApiCore.PlacementFormat { | ||
| static func from(size: CGSize?) -> BidMachineApiCore.PlacementFormat { | ||
| let height = size?.height ?? 50 | ||
| switch height { | ||
| case 50...89: | ||
| return .banner320x50 | ||
| case 90...249: | ||
| return .banner728x90 | ||
| case 250...: | ||
| return .banner300x250 | ||
| default: | ||
| return .banner320x50 | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
For adaptive banner support, the size should be calculated like it is here.
There was a problem hiding this comment.
Changed in this commit.
| log(.loadSucceeded) | ||
| loadCompletion?(.success([:])) ?? log(.loadResultIgnored) | ||
| loadCompletion = nil |
There was a problem hiding this comment.
For adaptive banner support, we should return the loaded ad size like we do here. If there's a way in the BidMachine API to get the size of the loaded ad from the view, we can use that, but if not, just using the requested size should be fine.
There was a problem hiding this comment.
Changed in 570f70831c5e1ae8389c7c1eb446111cc00c7ad7
|
After contacting BidMachine, we now know it's safe to set the regulation info after init, so I removed all the storage code that was used to populate those values before init. |
Source/BidMachineAdapterConfig.swift
Outdated
|
|
||
|
|
| let chartboostMediationError = self.error(.loadFailureUnknown, error: error) | ||
| self.log(.loadFailed(chartboostMediationError)) | ||
| completion(.failure(chartboostMediationError)) |
There was a problem hiding this comment.
nit:
Inside a catch statement error should be non-optional, so we could pass the partner error directly.
| let chartboostMediationError = self.error(.loadFailureUnknown, error: error) | |
| self.log(.loadFailed(chartboostMediationError)) | |
| completion(.failure(chartboostMediationError)) | |
| log(.loadFailed(error)) | |
| completion(.failure(error)) |
| let chartboostMediationError = self.error(.loadFailureUnknown, error: error) | ||
| self.log(.loadFailed(chartboostMediationError)) | ||
| completion(.failure(chartboostMediationError)) |
There was a problem hiding this comment.
nit: same as in interstitial
| let chartboostMediationError = self.error(.loadFailureUnknown, error: error) | ||
| self.log(.loadFailed(chartboostMediationError)) | ||
| completion(.failure(chartboostMediationError)) |
There was a problem hiding this comment.
nit: same as in interstitial
|
|
||
| log(.showStarted) |
There was a problem hiding this comment.
Let's remove log(.showStarted) if there's no corresponding showSucceeded log anywhere
| log(.showStarted) |
|
I made all the mentioned changes in this commit. Building release candidate now, but need re-approval so this branch can be merged later. |
There's no Rewarded ad to test this with yet but Banner and Interstitial work.
I'm traveling the next four days but will address any feedback as soon as I return on Tuesday, and hopefully submit a release candidate soon after that.