Re: [review] waagent: Windows Azure Linux Agent (part1)
Hideki Yamane <henrich@debian.or.jp> writes:
Hi,
> Hi,
>
> reviewed source package roughly.
>
>
> -----------------
> README
>
>> Supported Linux Distributions
>> 1.CentOS 6.0+
>> 2.Ubuntu 12.04+
>> 3.Suse (SLES) 11SP2+
>> 4.Open Suse 12.1+
>
> Yes, let's add "5.Debian 6.0+" :-)
no, wheezy+. I really doubt that there's enough stuff on kernel side to
have things working on squeeze.
>
> And, it contains example source and too bit long.
> I think README should be divided into README, example.configuration.xml and
> example.topology.xml file, and add debian/example file and specify it for
> Debian package side.
Well, it's upstream README. The example can be extracted but I'm not
sure it's usefull. In most cases, one should only install the agent and
there's nothing else to do.
>
> -----------------
> debian/changelog
>
>> waagent (20120606-1) testing; urgency=low
>
> Upstream added its changelog and it says its version as 1.1, 1.2... see github.
> So, should update it to master and specify 1.2~git20121206-1 or so. (1.2 is not
> released yet, so added ~git20121206. 1.2 is greater than 1.2~git20121206)
This version of the agent has been taken from github before that any
versionning scheme has been choosen by upstream (it's from 6th of june
and 1.1 has been released on the 9th of november). I've made a 1.1-1
yesterday and will update it when the 1.2 is there.
In fact, I'm not even sure that the current changelog entries should be
kept before uploading to the archive so it'll be 1.1-1 or 1.2-1 with the
usual "Initial release (Closes: )" line.
>
> and don't specify "testing", you should set "unstable" or "experimental"
> if you don't have special reason (i.e. release managers ask you)
I know. It's testing because my repo is for testing. It'll be fixed
before uploading in the archive.
>
>
>> * Initial release
>
> Please submit ITP request to BTS and get its number, then put it as (Closes: #nnnnnn)
>
will do today.
>
> -----------------
> debian/control
>
>> Build-Depends: debhelper (>= 8.0.0), python
>
> python-all is better than python.
>
>
>> Standards-Version: 3.9.3
>
> Now newest version is 3.9.4. Probably lintian warn it's too new but you can
> safely ignore it.
>
>> Vcs-Git: git://github.com/Windows-Azure/WALinuxAgent.git
>> Vcs-Browser: https://github.com/Windows-Azure/WALinuxAgent/
>
> No, Vcs-* field should point Debian package vcs, not upstream one.
> Set "Homepage: https://github.com/Windows-Azure/WALinuxAgent/"; instead.
>
>
> -----------------
> debian/rules
>
>> # -*- makefile -*-
>
> remove it :)
>
doesn't hurt to keep it too :)
>> install-agent:
>> mkdir -p debian/tmp
>> cp waagent README NOTICE debian/tmp
>>
>>
>> .PHONY: install-agent
>> override_dh_auto_install: install-agent
>
> This target is unnecessary. Just specifing those files in debian/{install,docs}
> is enough. And, I wonder that including NOTICE file or not, since it just says
> "Copyright 2012 Microsoft Corporation" and it is described in debian/copyright.
>
>
> -----------------
> debian/copyright
>
>> Copyright: 2012 Microsoft <walinuxagent@microsoft.com>
>
> Maybe "Microsoft Corporation" is better.
>
>
>> Files: debian/*
>> Copyright: 2012 Arnaud Patard <apatard@hupstream.com>
>> License: GPL-2+
>
> Well, same license as upstream source for debian/* is better, in my opinion.
> If debian/patches/* is distributed under GPL, can you apply it to Apache-2.0
> source?
oh. I meant debian/* but not patches. It'll be empty soon so I have not
taken it into account. I'll add an entry for debian/patches to make
things clearer.
Thanks,
Arnaud
Reply to: