[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Request to review and upload libewf 20140813-1



Hello Daichi, I'm dropping Sebastian from the thread as I don't think
he's interested in this,

>> Thanks a lot for the feedback.
>> Plus, apologies for my very late reply.

No worries about the delay, as you can notice, I might also take a bit
to reply back so it's all fine.

>> > 1) d/changelog:
>> That's right.
>> It' just that I forgot to remove the Closes stanza from my draft.
> I removed that stanza from the d/changelog draft.

Nice, I have a few comments about d/changelog in general so I'll write it here:
4) debian-changelog-line-too-long:
Lintian is complaining about line number 9 being too long, so that
line needs to be broken into two.

5) no-nmu-in-changelog and source-nmu-has-incorrect-version-number:
Lintian thinks this is an NMU upload because there's no "* Team
upload" line in the changelog. Please add it as the very first line in
the changelog entries (it doesn't need to be under anyone's name, in
other words, this can be added to line number 3).

6) spelling-error-in-changelog Debian Debian (duplicate word):
This is a false-positive from lintian as it doesn't properly parse the
"Debian Janitor" entry, this will go away when Lintian addresses the
issue or a new changelog entry is made. You can ignore it, but if you
want to perform a workaround for it, you can swap line 4 with line 5
(breaking the "debian... debian" chain in the changelog).

>> > 2) d/tests:
>> It seems very useful to use autopkgtest-pkg-python for this kind of testing.
>> Yes, I'll update d/control as you suggested and see it's enough for our test case.
>> Thanks for introducing that package.
> autopkgtest-pkg-python was added to d/control.
> An autopkgtest job successfully passed in CI:
> https://salsa.debian.org/dfukui/libewf/-/pipelines/398401

Perfect.

>> > 3) d/copyright:
>> licensecheck reports a bunch of source files that are subject to other licenses than LGPL3+.
>> It may need much effort to review that license scan report and update d/copyright accordingly.
>> So I'd appreciate it if you allow me to take some time.
> New files, copyright holders, and licenses were added to d/copyright.
> To review how things are changed, take a look at the following commit:
> https://salsa.debian.org/dfukui/libewf/-/commit/c2fcbfd76ec8705765fa8e0e6bc85beade3a4cfe

Cool, a few problems arose from those changes, most of these are
coming from Lintian, double check if you can see them in your dev
environment too:

7) bad-exception-format-in-dep5-copyright expat with public-domain:
We usually use the "foo with bar" pattern to show that there's license
exception in place, which is not the case here, the code is instead
slightly dual licensed (the FSF contributions are public-domain).
I suggest changing the license name to "Expat and public-domain".

8) invalid-short-name-in-dep5-copyright:
There are a couple of licenses with the wrong name in d/copyright, you
can read the lintian's output to find all of them. The lintian's
finding description also points to the documentation which has a table
of known license names.
Let me know if you have trouble finding any of that and I will help you.

9) license-file-listed-in-debian-copyright COPYING:
As per the finding's full description, this can be removed.

10) Files: debian/*:
You can add your name to this entry, and also the names of anybody
else who contributed to the package. I'll leave it up to you to decide
whether or not to do it as I don't consider this critical and I know
d/copyright stuff can be too time consuming.
The lintian finding "update-debian-copyright 2015 vs 2022" is about this.

11) Files: manuals/ewfinfo.1 ...:
The whole entry that lists the manuals/* files can be removed as they
all match under the "Files: *" entry and have the same license
details.

12) unused-license-paragraph-in-dep5-copyright gpl3+ [debian/copyright:133]:
There's no file with "License: GPL3+", so the license paragraph can be
removed. You can read the lintian finding in its entirety if you need
more details.

13) Copyright: Copyright (C):
You don't need to add "Copyright (C)" to the "Copyright" field of
d/copyright, that can be removed.

> Meanwhile, a new tag debian/20140813-1 was also added:
> https://salsa.debian.org/dfukui/libewf/-/tags/debian%2F20140813-1

Great, the debian/ tag is not generally needed by the way, as the
sponsor is the one who should push that ideally, but I don't mind if
you push it. I know gbp will work smoother if you create a debian/ tag
(it lets you do gbp push without extra parameters if there's a tag).

Some extra notes now:

12) older-debian-watch-file-standard 3:
You can bump d/watch to version 4.

13) out-of-date-standards-version 4.5.0:
You can bump Standards-Version to the latest one if you go through the
upgrading checklist and consider the package is compliant with the
latest version:
https://www.debian.org/doc/debian-policy/upgrading-checklist.html

14) silent-on-rules-requiring-root:
You can check if Rules-require-root is needed and set the value
accordingly in d/control. The way I like to check is by performing two
builds, one with each value, and then comparing the binaries with
diffoscope, if they're the same, then root is not needed.

15) debian-rules-uses-as-needed-linker-flag:
That flag can be removed from d/rules, as indicated by lintian.

16) typo-in-manual-page:
Lintian is detecting a few typos, this might be a good opportunity for
you to send patches upstream fixing those, but this is definitely not
required for the upload to be made.

I've tried to do a more thorough review as you seem interested in
learning all of it. Please let me know if you'd like me to only
mention what is required (which would be totally fine as you're
contributing and whatever you do is still good for Debian).

On a high level, I suggest always making sure lintian gets run on your
build process, preferably with the following parameters: '-i', '-I',
'-E', '--pedantic'.
You don't need to solve all lintian findings, some of them might not
make much sense or just not be doable at all, but it's good to try to
understand at least what's critical so you can make a judgement call.

Let me know if you'd like help on any of this,

Thanks for contributing,

-- 
Samuel Henrique <samueloph>


Reply to: