> = Summary = > Mechanical 3.0 changes for DistroArchSeriesBinaryPackage[Release] pages These pages look *so* much better! It's great seeing LP transformed :) > > == Implementation details == > I slipped in a quick bug fix while I was at it: > https://bugs.edge.launchpad.net/bugs/241341 > to fix a page heading. This was just a case of changing the title property on > the context class. Great! As mentioned in irc, you've added an extra 'for' into the title unintentionally, and it would be good to use smartquotes here. > > The change to the DistroArchSeriesBinaryPackage is trivial and can be seen > here: > https://launchpad.dev/ubuntu/warty/i386/mozilla-firefox > > The change to the DistroArchSeriesBinaryPackageRelease page was more involved. > It uses several page fragments in the package relationships section that are > in a different context/view. They are not used anywhere else so I added some > conditional "yui-u" div classes to those so that they don't appear when not > necessary. > > The breadcrumbs on both pages are not working properly, I will fix those in a > separate branch. > > == Tests == > bin/test -vvt stories.soyuz -t distroarchseriesbinarypackage Looks like the tests for the title need updating (or you may have done it since I merged). The only other thing is that we need to ensure we markup the package relationships correctly... currently there are dt's that are not enclosed by dl's (like having an

without it being inside a

is then followed by a

's, but we can't fix that yet as another template (sourcepackage-index.pt) uses the same +render-list traversal (which in-turn uses the packagerelationship-list.pt). So I think we should wait until sourcepackage-index.pt is updated to 3-0 before updating packagerelationship-list.pt to output dd's). I'll leave it up to you whether it's worth an XXX. Looking forward to seeing it land! IRC log: great bigjools: you mention in your MP that the breadcrumbs aren't working properly - in what way? noodles775: they are now, I landed a change last night :) bigjools: heh, I couldn't find a problem :) they stopped at 9.04 bigjools: any reason you didn't use smartquote in the title? or whatever version meh, details :) (like in model/distributionsourcepackage's title?) bigjools: also, see https://launchpad.dev/ubuntu/warty/i386/mozilla-firefox What do you think we should do in that case? (The empty Package relationships section) noodles775: it will never be empty in real life Great. our sample data is bong * jtv (n=jtv@125.25.88.144.adsl.dynamic.totbb.net) has joined #launchpad-reviews bigjools: also with the dasbp title, was it an explicit decision to do '"mozilla-firefox" binary package in Ubuntu Karmic for i386' rather than without the 'for' as suggested by mpt: '"mozilla-firefox" binary package in Ubuntu Karmic i386' noodles775: it was not, I missed that well spotted bigjools: Do you think it would be worthwhile moving all those relationship portlets inline in the dasbpr index? From here it looks like it would not only make maintenance easier but would reduce the line-count? noodles775: we can't do that they're in a different context Why's that? I thought they weren't re-used? hrm. nothing to do with re-use :( I'm not yet understanding... wouldn't tal:define="relationships view/breaks" just become... oh, right, separate view. exactly we could potentially do it by moving stuff to the dasbpr view but that's more work than I want to take on for this change yep, I think it'd be a good cleanup, but agree - another branch :) ideally, they'd be macros in the page template bigjools: We're currently mixing two different list styles for those relationships... I notice that packagerelationship-list is also used by sourcepackage-index.pt :/ So ideally, it should be outputting a bunch of dd's for this current template, but that would then leave the sourcepackage-index template with mixed list types. * bigjools checks Rather than

term: first; second

it's doing

term

I'm not sure what the best thing to do is... probably to leave your template as is, but put an XXX in the packagerelationship-list.pt to a bug for updating the sourcepackage-index template, identifying that they should be updated to dd's when that template is updated (for consistency - currently the sourcepackage-index template uses h3 followed by a ul). I'm still trying to find a page that shows the error bigjools: there is no error visually, it's just that the lists semantically do not make sense... ok I'm not even sure that it would validate (ie. having

etc.) * bigjools is being thick But we could fix it when the sourcepackage-index.pt is updated? What do you think? still trying to get my head around where the problem is A more obvious example would be ...

- a tr cannot be inside a td... it doesn't make sense. Similarly, when a browser sees a

's followed by one or more: 's, not a
followed by a . * deryck (n=deryck@samba/team/deryck) has joined #launchpad-reviews noodles775: I can't see where you mean how is sourcepackage-index.pt affected? it uses the same +render-list that all these relationship templates do, afaics? bigjools: I'll do a diff as part of my reply - hopefully that'll make more sense. are you sure? bigjools: I think so... I find it hard to explain things with just irc :), where as a diff is much more precise :) I'm being super thick ah * bigjools found it Great! I'm approving it now (just noting the things we discussed). noodles775: actually won't it be ok? I use bigjools: in the package relationships section? in the portlets sorry in the detail portlet nowhere else uses a dl it's a lone dt yeah, that one is fine, it's the lists in the package relationships section, like this: http://pastebin.ubuntu.com/264322/ Yes, that's part of the problem. really? thought it would be ok A dt should only ever exist inside a dl (afaik) ok so shall I just make it ? bigjools: You could - but then you'd need to ensure that it gets a new-line break etc... I think semantically a dl is the right way to go (as we have else-where with the 3-0 templates). I'm doing a quick diff now... okidoki bigjools: so it's just a matter of doing this for each of the relationship portlets: http://pastebin.ubuntu.com/264324/ noodles775: ok, great! It doesn't affect the display. And we can update the packagerelationship-list.pt to print dd's when we do the only other callsite.: bigjools: in the package relationships section? in the portlets sorry in the detail portlet nowhere else uses a dl it's a lone dt yeah, that one is fine, it's the lists in the package relationships section, like this: http://pastebin.ubuntu.com/264322/ Yes, that's part of the problem. really? thought it would be ok A dt should only ever exist inside a dl (afaik) ok so shall I just make it ? bigjools: You could - but then you'd need to ensure that it gets a new-line break etc... I think semantically a dl is the right way to go (as we have else-where with the 3-0 templates). I'm doing a quick diff now... okidoki bigjools: so it's just a matter of doing this for each of the relationship portlets: http://pastebin.ubuntu.com/264324/ noodles775: ok, great! It doesn't affect the display. And we can update the packagerelationship-list.pt to print dd's when we do the only other callsite.