intetsim review (Re: ITP: inetsim)
Lukas Schwaighofer
lukas at schwaighofer.name
Mon Nov 13 21:15:58 UTC 2017
Hi GengYu,
On Fri, 10 Nov 2017 15:53:36 +0000
GengYu Rao <zouyoo at outlook.com> wrote:
> On Tuesday, October 31, 2017 04:53 AM, Lukas Schwaighofer wrote:
> > After you've fixed all that, I should be done with my review and we
> > can ask one of the DDs here to take a look! :)
> I fixed all that, and there is an email from upstream
>
> The runtime data directory is/var/lib/inetsim/.
> Sample files for some services are included in/usr/share/inetsim/.
> Some of those files are copied by the postinst script
> to/var/lib/inetsim/ where they can safely be replaced/deleted.
>
> the upstream's package is somehow chaotic, and i put
> them all into /var/lib/inetsim/ in the install script.
> the upstream's deb is here.
> http://www.inetsim.org/debian/binary/inetsim_1.2.7-1_all.deb
> But i managed to get everything fine in my repo:)
I have some more feedback:
* You added quite a few patches in one commit (with the message "add
the upstream patches"). Please add a DEP-3 [1] compatible header to
those patches. The patches should include a link to their origin.
* A lot of files are now installed to both
/usr/share/inetsim/lib/ and /usr/share/perl5, they should only be
installed once.
* You changed the location for a lot of files, but the "old" (empty)
directories are still part of the package.
- The "report" symlink still points to /var/lib/inetsim/report .
* Please write the commit messages more carefully. It will be hosted
on a Debian server, so your work is public and, if you maintain a
package, it reflects on Debian:
- I find a commit message "the ugly install", noting that you are not
satisfied with the format of the install files, inappropriate. The
debhelper folks are making a lot of effort to make packaging as
easy as possible. If you have constructive feedback on how they
can improve something, you can discuss with them on their mailing
list.
- "got an email from upstream i will post it here" (followed by the
email) is not a good commit message either. Explain what you did
and add the information provided by upstream
- "this is important, please check previous commit message" is not
helpful
- If you have important notes for people handling the source (which
includes future maintainers), create a debian/README.source
file [2]. Commit messages are not a good place for that.
* It seems like default-disabled currently does not work as it should:
The status is not synced properly to systemd… I think for now it's
better to simply revert 62896682920aa5d56fc5dcf71d19dd2e5e3a225d and
keep the ENABLED=0, even though I don't like that…
Regards
Lukas
[1] https://dep.alioth.debian.org/deps/dep3/
[2] https://www.debian.org/doc/debian-policy/#source-package-handling-debian-readme-source
More information about the Pkg-security-team
mailing list