Revert "Launch no center css for ad container" #28844
Conversation
This reverts commit f2843ea.
|
Hey @Jiaming-X! These files were changed: Hey @jeffkaufman! These files were changed: |
|
Make sense. I am curiously how it can be caught so quick? Did someone from gq |
|
@keithwrightbos spotted it by coincidence earlier today. |
|
I specifically tested for this case, but must've missed it. I think on the page that I tested, the slot was already centered, so I must've not noticed that the (smaller) creative was slightly off center. |
|
@glevitzky I think we can simply do a fix forward. I don't think the experiment will give us more helpful insights, because the failure should have been caught but was not. |
|
Reason I want the experiment back is that it will prevent the bug from going out to 95% of traffic. Agreed that it won't give us any new info, though. My thinking is:
If we don't do (1) first, and go straight to (2), then we'll "break" multi-size creatives until the next code release. |
|
@glevitzky Yeah I agree we must prevent it from getting out now. I was thinking your |
|
Got it. I'll put together a separate PR, then. |
wasn't this out in last week's release via a flag flip? |
…pproject#28844) This reverts commit f2843ea.
Reverts #28551
Looks like this is causing an issue for multi-size creatives :\.
found on:
http://localhost:8000/proxy/s/www.gq.com/story/kanye-west-may-2020-cover-announcement/amp
The plan should be to revert the change here to status quo. After that, I will make a code change to apply centering logic to multi-size creatives only, and then we can re-apply the reverted CL.
Alternatively we can unlaunch only the experiment traffic fractions, and then roll-forward a fix (applying the centering logic to multi-size creatives).