Code review comment for lp://staging/~gary/launchpad/bug844631

Revision history for this message
Brad Crittenden (bac) wrote :

Thanks for the nice work Gary. I know the branch had lots of frustrations but I'm glad you got them all tamed.

I wonder if it would've been better to have split this effort into two branches: one for the page and another to add the status messages. Might've helped you with momentum.

On IRC we discussed improvements to the text of the page and I like the final version you wrote. I like the convivial nature of the page but it does need to read smoothly too.

I notice you put up the spinner statically which has the downside of being displayed permanently if JS is not enabled in the browser. I think it should be displayed conditionally. The header "Recent status updates" should also only be showed if there is a chance it will be populated.

Otherwise it looks good. You just mentioned on IRC you'll reinstate the publisher code.

Also, thanks for the lint cleanup.

review: Approve (code)

« Back to merge proposal