INN / doubleclick-for-wp

WordPress plugin for serving Google Ad Manager ads
https://wordpress.org/plugins/doubleclick-for-wp/
GNU General Public License v2.0
25 stars 11 forks source link

How should we deal with the new breakpoint settings? #15

Closed rnagle closed 7 years ago

rnagle commented 8 years ago

The breakpoint settings have changed in a way that will allow the plugin to be more responsive in the future. For example, it will allow ad placements that are 728x90 size to be replaced with a smaller banner on smaller screens.

However, the new settings will mean anyone using the DFP plugin will need to reconfigure at least some of their currently-deployed DFP widgets.

What are our options for making this as painless as possible?

benlk commented 7 years ago

We'd need to write a migration function, like how Link Roundups and Largo update.

Are there any parts of the migration that require human decisions, or can we automate this?

benlk commented 7 years ago

Per discussion in Asana, reproduced below, we're making this a breaking change:

@aschweigert:

how many people are actually using the plugin? I'm not that worried about it if it just means we have to go in and change a few options for a few sites

@benlk:

https://wordpress.org/plugins/doubleclick-for-wp/ claims 70+ installs>

There are 7 installs in the largoproject umbrella, RNS, Rivard, Cornell Sun, and Kinsey.

@aschweigert:

What would it take to write an update script for this (and for the plugin, generally)? I don't really have any way to cover/justify that cost so if it's going to be expensive, we're going to need to bite the bullet and just make the breaking change and note it in the release notes.

@benlk:

We have existing migration scripts for plugins, like the stuff for Link Roundups: https://github.com/INN/link-roundups/blob/master/inc/updates/index.php

The settings that will need to be migrated are in the widgets and in the plugin. The breakpoint settings remain the same, iirc, but what we do with them in the widget changes. We're moving from "display this ad unit on breakpoints X, Y, Z" to "at breakpoint X, display , at Y, display, at Z, display ". We have X, Y, and Z from the old settings, and we have from the old display size. I guess we can copy into all the breakpoints for all the widgets.

That will take a day, and it won't solve the problem of widgets now displaying inappropriate sizes at different resolutions. I can't think of any easy way to merge two widgets in widget area that were set to display at the different resolutions, so this would still require a human element.

Yeah, this is just going to have to be a breaking change. I'm in favor of making it version 1.x, because it is a breaking change.

We'll want to write docs for the update process, and do some testing to see what stays around after an update. Update-specific docs might take a day, but we're writing docs for this area already.

@aschweigert:

why would update docs take a day? don't we just need to go site by site (for our sites) and update a few settings? or am I missing something larger here?

@benlk:

You're not missing anything; I was thinking that we should provide strong update docs for non-us users so that their transition would be pretty low-stress. (I'm bad at estimating time for documentation, I guess)

@aschweigert :

ok, yeah, I think all we need is just a note in the release notes that there's a breaking change + a short description of what needs to be done to address this (so, maybe 30 minutes)

@benlk:

Is https://github.com/INN/DoubleClick-for-WordPress/blob/master/readme.txt#L36 sufficient?

@aschweigert:

yep

benlk commented 7 years ago

Updating this with a correction:

Production is at 0668f6e6907fac3f66bb6d97156a583b1d4a9742 The released version is 31b30900f1a6ac6195633f6445ad4ec8f6c1a6dc, several commits later

The released version contains https://github.com/INN/DoubleClick-for-WordPress/pull/3 , which is what I thought would cause a breaking change in non-INN users. It turns out that the 70+ users who installed from wordpress.org are already after the commit that would cause the change, so we're actually in the clear for them. We'll just need to plan migrations for the INN site.