Ubuntu Apps Directory

Merge lp://staging/~michael.nelson/ubuntu-webcatalog/962140-display-video-demos-pt-2 into lp://staging/ubuntu-webcatalog

  1. 962140-display-video-demos-pt-2
  2. Merge into trunk
Proposed by Michael Nelson
Status: Merged
Approved by: Łukasz Czyżykowski
Approved revision: 126
Merged at revision: 117
Proposed branch: lp://staging/~michael.nelson/ubuntu-webcatalog/962140-display-video-demos-pt-2
Merge into: lp://staging/ubuntu-webcatalog
Diff against target: 180 lines (+116/-4)
5 files modified
src/webcatalog/models/applications.py (+15/-4)
src/webcatalog/static/css/webcatalog.css (+9/-0)
src/webcatalog/templates/webcatalog/application_detail.html (+8/-0)
src/webcatalog/tests/test_models.py (+73/-0)
src/webcatalog/tests/test_views.py (+11/-0)
To merge this branch: bzr merge lp://staging/~michael.nelson/ubuntu-webcatalog/962140-display-video-demos-pt-2
Reviewer Review Type Date Requested Status
Canonical Consumer Applications Hackers Pending
Review via email: mp+104714@code.staging.launchpad.net

Commit message

Add videos to the application detail page.

Description of the change

Overview
========

Adds videos to the application detail page, to look like this:

http://people.canonical.com/~michaeln/tmp/962140-video-iframes.png

Note:
 * The video we have on staging is no longer allowed to be embedded (that's an option set by the author)
 * I've also tweaked the style for the sharing options so that the facebook options are aligned.

`fab test`

To post a comment you must log in.
Revision history for this message
Łukasz Czyżykowski (lukasz-czyzykowski) wrote :

Why using setattr, gettattr, hasattr for media caching instead of just plain dict (self._media_cache = {}). Or other than doing per request cache, maybe it would be better to push that to django cache layer?

lp://staging/~michael.nelson/ubuntu-webcatalog/962140-display-video-demos-pt-2 updated
126. By Michael Nelson

REFACTOR: update media cache to use a single dict rather than multiple private attributes.

Revision history for this message
Michael Nelson (michael.nelson) wrote :

On Fri, May 4, 2012 at 12:47 PM, Łukasz Czyżykowski
<email address hidden> wrote:
> Why using setattr, gettattr, hasattr for media caching instead of just plain dict (self._media_cache = {}). Or other than doing per request cache, maybe it would be better to push that to django cache layer?

12:02 < noodles> lukasz: +1 for self._media_cache instead, but I think
we'd specifically not want to use the django cache layer as caching
over multiple requests would be bad?
12:02 * noodles updates
12:03 < lukasz> noodles, why?
12:06 < noodles> lukasz: run import_for_pay_apps, then view a specific
app, but it's screenshots haven't updated, or multiple points in the
code need to know about the cache (to clear it during import)
12:06 < noodles> lukasz: I guess the second option isn't so bad.
12:07 < lukasz> noodles, yeah, that would be my only concern, a need
for keeping the cache up to date
12:07 < lukasz> noodles, ok, let's leave it at dict
12:07 < noodles> k
12:12 < noodles> lukasz: pushed with r126

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches