Review: Needs Fixing So, my feeling on this is each slideshow content version should be targeted towards a particular Ubuntu release. If it's necessary to grab the version ID automatically, my assumption is that means we're either shipping outdated content (because, really, a lot changes from one release to the next and we've already had a few instances where flavours ship with plainly incorrect slideshow content); or we're shipping very general content (which could be argued for…). Am I missing something? :)
Personally, I've always been an advocate of just not repeating the version number here at all since Ubiquity already communicates that and it isn't really relevant information at this stage, but I don't think I've won that argument. Anyway, this is optional, so I don't mind, but for this change to work, we'll also need a change in a released or to-be-released version of Ubiquity that is similar to what you have in Slideshow.py. Has that happened at this stage? It might also be nice to have a sensible fallback in the event that the version ID isn't provided, too. Right now it looks like we'll get an empty space after a comma, and no version number. Perhaps wrap that whole thing in an element that disappears if there's no version number available, like "Xubuntu<span class="versionDetails">, <span class="releaseName"></span> <span class="versionId"></span></span>"? Also - and I promise this is a tiny thing - your element classes are inconsistent. Please use dashes instead of camel case, there, like "release-name" and "version-id". Thank you! -- https://code.launchpad.net/~lderan/ubiquity-slideshow-ubuntu/test/+merge/180747 Your team Ubiquity Slideshow is requested to review the proposed merge of lp:~lderan/ubiquity-slideshow-ubuntu/test into lp:ubiquity-slideshow-ubuntu. _______________________________________________ Mailing list: https://launchpad.net/~ubiquity-slideshow Post to : [email protected] Unsubscribe : https://launchpad.net/~ubiquity-slideshow More help : https://help.launchpad.net/ListHelp
