r20940 - in /packages/unstable/update-manager/debian: ./ patches/ pos/

Sun Aug 16 18:52:35 UTC 2009

Hi Emilio,

Comments inline.

On Sun, 2009-08-16 at 17:57 +0200, Emilio Pozuelo Monfort wrote:
> speijnik-guest at users.alioth.debian.org wrote:
> > --- packages/unstable/update-manager/debian/changelog [utf-8] (original)
> > +++ packages/unstable/update-manager/debian/changelog [utf-8] Sun Aug 16 09:01:59 2009
> > @@ -1,15 +1,15 @@
> > -update-manager (0.68.debian-8) UNRELEASED; urgency=low
> > +update-manager (1:0.200.0~exp3) UNRELEASED; urgency=low
> 
> You don't need the epoch here.

I added the epoch because old upstream used the epoch for the 0.1xx.x
versions. I assumed this would make it easier for both Ubuntu and Debian
to keep track of versions.
Without the epoch Ubuntu is likely going to use 1:version, whilst Debian
isn't using it.

> > +  * New upstream release. Closes: #515965, #529333, #480504, #449040,
> > +    #519191, #454686, #462188, #503584, #519320, #541584, #533994,
> > +    #415376.
> 
> It's a good practice to say what is fixed for each bug. E.g.

See comment about UNRELEASED below.

>   * No longer does foo. Closes: #123.
>   * Fixed foobar to work with baz. Closes: #124.
>   * ...
> 
> > +  * Complete rework of update-manager.
> > +  * Automatic bug reporting via reportbug.
> > +  * Synaptic not needed anymore, replaced with python-apt.
> > +  * Somewhat experimental, please report all problems.
> > +  * NOT RELEASED YET.
> 
> You have UNRELEASED in the distribution, this is not needed :)

I added UNRELEASED for now just to mark it as "not ready for an upload
yet". This is common practice within the python modules team.

> > Modified: packages/unstable/update-manager/debian/control
> > URL: http://svn.debian.org/wsvn/pkg-gnome/packages/unstable/update-manager/debian/control?rev=20940&op=diff
> > ==============================================================================
> > --- packages/unstable/update-manager/debian/control [utf-8] (original)
> > +++ packages/unstable/update-manager/debian/control [utf-8] Sun Aug 16 09:01:59 2009
> 
> You should modify debian/control.in and update debian/control with 'fakeroot
> debian/rules clean'. Otherwise you will loose your changes when debian/control
> is updated from control.in.

Point taken. I'll have a look into that. My commit wasn't meant to be the final commit anyways. Just so you can see I'm making progress. :-)

> > +Vcs-Browser: http://svn.debian.org/viewsvn/pkg-gnome/packages/unstale/update-manager
>                                                                  ^^^^^^^
> s/unstale/unstable/

Typo, will be fixed in next commit, thanks for pointing that one out.

> > -Package: update-manager
> > +Package: update-manager-core
> > +Architecture: all
> > +Section: admin
> > +XB-Python-Version: ${python:Versions}
> > +Depends: ${python:Depends}, 
> > +	 ${misc:Depends}, 
> > +	 lsb-release, 
> > +	 update-manager-gnome (= ${source:Version}), 
> 
> update-manager-core depends on update-manager-gnome, and viceversa. I think
> that's problematic. I'd remove the update-manager-core dependency, and let
> update-manager-gnome (and other frontends when they are written) depend on the
> backend, to break the dependency loop.

Ok, I'll break that. The reason I added the dependency on the frontend
is because the core alone won't be of any use. Originally this was a
dependency on update-manager-gnome | update-manager-text. See my comment
about the -text frontend below.

> > +	 python-apt (>= 0.7.11.1), 
> > +	 gksu
> > +Replaces: update-manager (<< 1:0.93.7)
> > +Conflicts: update-manager (<< 1:0.93.7)
> > +Description: manage release upgrades
> > + This is the core of update-manager
> 
> The description could be a bit more concrete. I'd put a description of what
> update-manager is and what it does in all the description, and after that I'd
> put what each package contains or does.

That's on my TODO list. :-)

> > +Package: update-manager-apidoc
> 
> I'd name the package update-manager-doc, to follow common practice.

Ok, point taken.

> > +Architecture: any
> 
> Shouldn't this be all?

Again, you are right there. That needs to be fixed.

> > +Section: doc
> > +Depends: ${misc:Depends}
> > +Suggests: libjs-jquery, 
> > +	  update-manager-core (= ${source:Version}), 
> > +	  update-manager-gnome (= ${source:Version})
> > +Description: Update Manager API documentation
> > + This package contains the API documentation for Update Manager. Please note
> > + that this is only useful if you intend to work on Update Manager and does
> > + not include any documentation targeteted at end-users.
> 
> s/targeteted/targeted/

Typo, again. :-)

> > Modified: packages/unstable/update-manager/debian/copyright
> > URL: http://svn.debian.org/wsvn/pkg-gnome/packages/unstable/update-manager/debian/copyright?rev=20940&op=diff
> > ==============================================================================
> > --- packages/unstable/update-manager/debian/copyright [utf-8] (original)
> > +++ packages/unstable/update-manager/debian/copyright [utf-8] Sun Aug 16 09:01:59 2009
> > @@ -1,61 +1,108 @@
> >  This package was debianized by Michiel Sikkes <michiel at eyesopened.nl> on
> >  Mon, 25 Oct 2004 21:49:07 +0200.
> >  
> > -It was downloaded from http://ftp.ubuntu.com/ubuntu/pool/main/u/update-manager/
> > +It was downloaded from http://alioth.debian.org/frs/?group_id=100374
> >  
> >  Upstream Authors:
> >  	 Michiel Sikkes <michiel at eyesopened.nl>
> >  	 Michael Vogt <mvo at debian.org>
> >  	 Martin Willemoes Hansen <mwh at sysrq.dk>
> > +         Stephan Peijnik <debian at sp.or.at>
> 
> Indentation? ;)

True. ;-)

> > Modified: packages/unstable/update-manager/debian/rules
> > URL: http://svn.debian.org/wsvn/pkg-gnome/packages/unstable/update-manager/debian/rules?rev=20940&op=diff
> > ==============================================================================
> > --- packages/unstable/update-manager/debian/rules [utf-8] (original)
> > +++ packages/unstable/update-manager/debian/rules [utf-8] Sun Aug 16 09:01:59 2009
> > @@ -1,49 +1,65 @@
> >  #!/usr/bin/make -f
> >  
> > -DEB_PYTHON_SYSTEM := pysupport
> 
> Why are you switching to pycentral when everybody is switching to pysupport?

Opps, I missed that. I didn't want to use pycentral.

> > -include /usr/share/gnome-pkg-tools/1/rules/uploaders.mk
> 
> You're removing our team magic!

Is this really needed?

> > Modified: packages/unstable/update-manager/debian/update-manager-core.install
> > URL: http://svn.debian.org/wsvn/pkg-gnome/packages/unstable/update-manager/debian/update-manager-core.install?rev=20940&op=diff
> > ==============================================================================
> > --- packages/unstable/update-manager/debian/update-manager-core.install [utf-8] (original)
> > +++ packages/unstable/update-manager/debian/update-manager-core.install [utf-8] Sun Aug 16 09:01:59 2009
> > @@ -1,4 +1,20 @@
> >  debian/tmp/usr/share/locale
> > -debian/tmp/usr/lib/python*/site-packages/UpdateManager/Core
> > -debian/tmp/usr/lib/python*/site-packages/UpdateManager/fdsend.so
> > -debian/tmp/usr/lib/python*/site-packages/UpdateManager/__init__.py
> > +debian/tmp/usr/share/update-manager/bug_script
> > +debian/tmp/usr/lib/python*/*-packages/UpdateManager/__init__.py
> > +debian/tmp/usr/lib/python*/*-packages/UpdateManager/Application.py
> > +debian/tmp/usr/lib/python*/*-packages/UpdateManager/Config.py
> > +debian/tmp/usr/lib/python*/*-packages/UpdateManager/Exceptions.py
> > +debian/tmp/usr/lib/python*/*-packages/UpdateManager/ThreadManager.py
> > +debian/tmp/usr/lib/python*/*-packages/UpdateManager/Util/
> > +debian/tmp/usr/lib/python*/*-packages/UpdateManager/Backend/__init__.py
> > +debian/tmp/usr/lib/python*/*-packages/UpdateManager/Backend/loader.py
> > +debian/tmp/usr/lib/python*/*-packages/UpdateManager/Backend/PythonApt.py
> > +debian/tmp/usr/lib/python*/*-packages/UpdateManager/DistSpecific/__init__.py
> > +debian/tmp/usr/lib/python*/*-packages/UpdateManager/DistSpecific/changelog.py
> > +debian/tmp/usr/lib/python*/*-packages/UpdateManager/DistSpecific/loader.py
> > +debian/tmp/usr/lib/python*/*-packages/UpdateManager/DistSpecific/Auto.py
> > +debian/tmp/usr/lib/python*/*-packages/UpdateManager/Frontend/__init__.py
> > +debian/tmp/usr/lib/python*/*-packages/UpdateManager/Frontend/loader.py
> > +debian/tmp/usr/lib/python*/*-packages/UpdateManager/Frontend/GtkCommon/
> > +debian/tmp/usr/lib/python*/*-packages/UpdateManager/DistSpecific/Debian/
> > +debian/tmp/usr/lib/python*/*-packages/UpdateManager/DistSpecific/Ubuntu/
> 
> Maybe you should write a setup.py script upstream? ;)

setup.py exists, but I need that to break up the code of the package.
The main reason here is that UpdateManager/Frontend/Gtk/* should go into
the update-manager-gnome package, whilst all other code belongs to the
core.

> > Added: packages/unstable/update-manager/debian/update-manager-text.install
> > URL: http://svn.debian.org/wsvn/pkg-gnome/packages/unstable/update-manager/debian/update-manager-text.install?rev=20940&op=file
> > ==============================================================================
> > --- packages/unstable/update-manager/debian/update-manager-text.install (added)
> > +++ packages/unstable/update-manager/debian/update-manager-text.install [utf-8] Sun Aug 16 09:01:59 2009
> > @@ -1,0 +1,2 @@
> > +debian/tmp/usr/bin/update-manager-text
> > +debian/tmp/usr/lib/python*/*-packages/UpdateManager/Frontend/Newt.py
> 
> update-manager-text? That is not listed in debian/control

Well, update-manager-text is not ready for primetime yet. It will be
included in later releases, but for now I concentrated in making the Gtk
frontend work as intended. 

Basically the text/newt frontend wasn't updated in a while now and
didn't work the way I wanted it to work anyways (ie. suspending the newt
screen and doing simple text I/O for progress, when there is a way of
showing a progress bare in newt).

> 
> That's all for now, need to try the new update-manager next :)

It should work quite well and ask you to report a bug when something
breaks (read: uncaught exception). 

Thanks again for your input,

Stephan