Inkscape-website

Merge lp://staging/~doctormo/inkscape-web/inkscape-web-inkpicture into lp://staging/inkscape-web

inkscape-web-inkpicture
Merge into inkscape-web

Proposed by Martin Owens on 2016-09-07

Status:	Merged
Merged at revision:	1432
Proposed branch:	lp://staging/~doctormo/inkscape-web/inkscape-web-inkpicture
Merge into:	lp://staging/inkscape-web
Diff against target:	420 lines (+294/-8) 11 files modified cmsplugin_image/__init__.py (+3/-0) cmsplugin_image/cms_plugins.py (+50/-0) cmsplugin_image/migrations/0001_initial.py (+34/-0) cmsplugin_image/migrations/0002_auto_20160907_2044.py (+35/-0) cmsplugin_image/migrations/0003_auto_20160907_2241.py (+23/-0) cmsplugin_image/models.py (+95/-0) cmsplugin_image/templates/cms/plugins/picture.html (+18/-0) cmstabs/models.py (+1/-1) inkscape/settings.py (+1/-1) inkscape/static/css/main.css (+34/-5) utils/requirements.txt (+0/-1)
To merge this branch:	bzr merge lp://staging/~doctormo/inkscape-web/inkscape-web-inkpicture
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Hachmann (community)		2016-09-07	Approve on 2016-09-07
Review via email: mp+305148@code.staging.launchpad.net

Description of the change

Fixed up the previous merge request and make sure the initial migration keeps existing data from the djangocms_picture plugin.

Revision history for this message

Hachmann (marenhachmann) wrote on 2016-09-07:

Beside the missing closing ) in the comment on the plugin, it looks almost perfect.

The 'longdesc' field has not been removed (as the migration thinks), it has been renamed to what it is: title.

This does loose data on the website, as I added titles (at least) on the Sponsors page (how did you check for data loss?). So I guess it will need to be named back to its original name, and only the 'verbose' and 'help text' parts need to be changed.

Why is compatibility with the old plugin important? I liked the name ;-)

Revision history for this message

Hachmann (marenhachmann) wrote on 2016-09-07:

Now I'm not sure if you would like to make those changes yourself, or what the function of this merge request is...

Revision history for this message

Martin Owens (doctormo) wrote on 2016-09-07:

picture_plus was fine, I'm just being opinionated about the name of the python module. The cmsplugin name needed to stay PicturePlugin to keep all existing plugins plugged-in.

longdesc can be re-named in 0002 as that will incurr data loss. I'll fix these things now. Thanks for the reivew.

lp://staging/~doctormo/inkscape-web/inkscape-web-inkpicture updated on 2016-09-07

1434. By Martin Owens on 2016-09-07: Add extra migration for table rename

Revision history for this message

Martin Owens (doctormo) wrote on 2016-09-07:

OK should be updated now with a new migration to rename the table too.

Revision history for this message

Hachmann (marenhachmann) wrote on 2016-09-07:

Weird as this may seem, I guess I'm supposed to do the merge now...

Revision history for this message

Hachmann (marenhachmann) wrote on 2016-09-07:

Thanks for the changes, Martin.

Revision history for this message

Hachmann (marenhachmann) wrote on 2016-09-07:

Oh, you didn't only rename the picture, you replaced it.... :/ I hope you won't mind if I put it back in.

Revision history for this message

Hachmann (marenhachmann) wrote on 2016-09-07:

(sorry, wrong place, had both tabs still open)

Revision history for this message

Hachmann (marenhachmann) wrote on 2016-09-07:

Correction, the image is not replaced. But it doesn't show.

Revision history for this message

Hachmann (marenhachmann) wrote on 2016-09-07:

collectstatic, don't answer.

Revision history for this message

Hachmann (marenhachmann) wrote on 2016-09-07:

And wrong name of image... but still doesn't show. Sigh...

Revision history for this message

Hachmann (marenhachmann) on 2016-09-07:

review: Approve

Revision history for this message

Martin Owens (doctormo) wrote on 2016-09-08:

I wasn't expecting the actual merge. But I'm happy to have you have the experience.

Sorry for the unclarity in all of this. I'm really very happy with this contribution and your work on it :-)

Revision history for this message

Hachmann (marenhachmann) wrote on 2016-09-08:

It was just copy-pasting something that already existed. Those annoying migrations were probably correct from the start... (minus the removal, I must have had that, too).

The fun part was actually making the little icon. And thinking that Tav now has one reason less to complain about the website.