intetsim review (Re: ITP: inetsim)
- Subject: intetsim review (Re: ITP: inetsim)
- From: lukas@schwaighofer.name (Lukas Schwaighofer)
- Date: Mon, 13 Nov 2017 22:15:58 +0100
- Message-id: <[🔎] 20171113221558.6bc35091@localhost>
- In-reply-to: <MWHPR04MB1119E32FF163DC4B9EA69FE6C5540@MWHPR04MB1119.namprd04.prod.outlook.com>
- References: <MWHPR04MB11194760C1AFF88D89819344C5740@MWHPR04MB1119.namprd04.prod.outlook.com> <57e904d1-e3ba-c8dc-d866-ed9b283c12c8@inetsim.org> <MWHPR04MB11191E2EE74ACF63861099F9C5420@MWHPR04MB1119.namprd04.prod.outlook.com> <e51af857-a3b9-4abe-a3fd-5152fad344c7@inetsim.org> <MWHPR04MB1119CC89D84BA53D889783D3C55A0@MWHPR04MB1119.namprd04.prod.outlook.com> <20171029135006.1a3ef46f@localhost> <MWHPR04MB1119545B1616C320D6007A01C5590@MWHPR04MB1119.namprd04.prod.outlook.com> <20171030215318.29efd1c2@localhost> <MWHPR04MB1119E32FF163DC4B9EA69FE6C5540@MWHPR04MB1119.namprd04.prod.outlook.com>
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
Reply to: