Code review comment for lp:~qqworini/ubuntu-rssreader-app/new-gridview2

On Mon, Sep 2, 2013 at 10:11 AM, Joey Chan <email address hidden> wrote:

> Hi David,
> 1. do I need to do something for the autopilot test?
>

Hi Joey,

I've ask Nick Skaggs, our QA Comunity Coordinator to help you on this. You
can also find him on IRC as 'balloons' in the #ubuntu-app-devel channel on
Freenode.

> 2. I did try to fix conflicts in that branch, but I got some errors from
> "Bazaar Explorer", a bzr GUI client. then, I uploaded it to a new branch.
> Maybe my revision is too old to merge :P
>

No worries, no need to change anything now, I just thought I'd mention it
to make your life easier.

Roman, if you're happy with the code after your review, please feel free to
top-approve so that the merge proposal can land (to top-approve, just click
on the Needs Review link at the top of the merge proposal's page).

>
> > Hi Joey, really nice work!
> >
> > Just two quick comments:
> >
> > - There is an autopilot test that is failing. I believe either the test
> or
> > Jenkins needs updating to clear the database on each run, as looking at
> the
> > autopilot video [1] it seems it fails trying to create a topic that it
> already
> > exists
> > - To make your life easier for next time, there is no need to create a
> new
> > branch and a new merge proposal when you fix conflicts. The procedure is
> > simply.
> > 1. bzr merge lp:ubuntu-rss-reader-app to merge the latest trunk
> > 2. Fix any merge conflicts, either automatically or manually
> > 3. Commit and push the new revisions
> > 4. Then the merge proposal is automatically updated for you
> >
> > [1]
> http://91.189.93.70:8080/job/generic-mediumtests/132/artifact/ubuntu_rssre
> >
> ader_app.tests.test_rssreader.TestMainWindow.test_add_remove_feed_and_topic%20
> > (with%20mouse).ogv
> --
>
> https://code.launchpad.net/~qqworini/ubuntu-rssreader-app/new-gridview2/+merge/183377
> Your team Ubuntu RSS Feed Reader Developers is requested to review the
> proposed merge of lp:~qqworini/ubuntu-rssreader-app/new-gridview2 into
> lp:ubuntu-rssreader-app.
>