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

Hi, David!

Thanks for your tips ;)

2013/9/3 David Planella <email address hidden>

> 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.
> >
>
> --
>
> https://code.launchpad.net/~qqworini/ubuntu-rssreader-app/new-gridview2/+merge/183377
> You are reviewing the proposed merge of
> lp:~qqworini/ubuntu-rssreader-app/new-gridview2 into
> lp:ubuntu-rssreader-app.
>