Code review comment for lp://staging/~stefano-palazzo/gpxviewer/gpxviewer-null-pub

Revision history for this message
Andrew Gee (andrewgee) wrote :

Hi Stefano,

Thanks for the merge request. Looks like good work, from a quick glance.
I've been a bit busy this weekend, but should be able to review the code
later today or tomorrow and merge it in.

Thanks for your contribution :)

Andrew Gee

On 06/11/10 22:48, Stefano Palazzo wrote:
> Stefano Palazzo has proposed merging lp:~stefano-palazzo/gpxviewer/gpxviewer-null-pub into lp:gpxviewer.
>
> Requested reviews:
> Andrew Gee (andrewgee)
>
>
> I've notices that, when clicking on "Open with external Program..", nothing happens when
> a) the application is not installed
> b) no file is opened
>
> So I've changed the code to:
> a) ask the user if they want to install the application, if so, install it (transparently, using apt://..)
> b) open the application without sending it a file name to open
>
> I've also replaced os.spawn with subprocess.Popen (not that that's important, but it's recommended)
>
> If you're happy with those changes, I think they might be a nice addition to this fantastic application.

« Back to merge proposal