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

Bug#946142: [PATCH] log files for visibility by dh_missing (Closes: #946142)



On Mon 2019-12-30 07:50:42 -0400, David Bremner wrote:
> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>
>> These changes are inspired by the recommendations in "Logging helpers
>> and dh_missing" in /usr/share/doc/debhelper/PROGRAMMING.gz, and
>> derived from the source of dh_installman and dh_installinfo.
>>
>> (The weird extraction of the file list from @action is idiosyncratic
>> to dh_elpa, afaict)
>
> I guess you mean @actions?

yes, that's what i meant, sorry for the confusion.

> I had to read this a few times to understand that you meant something
> along the lines of "the list of source files is reverse engineered
> from the code just below" rather than "Hell is other people's code". I
> does seem a bit fragile, but I don't see an obviously better way to do
> it. It is a pity log_installed_files can only be called once.

Yep, i don't see a better way to do it either, but my perl is rusty
enough that i had to run a modified version of dh_elpa a few times and
look at some Data::Dumper output to produce this particular
construction, and none of the other perl bits of dh seem to assemble the
temporary data structure that dh_elpa is assembling here (and from which
i had to extract a list), which is what i meant by "idiosyncratic to
dh_elpa".

I certainly didn't mean "Hell is other people's code" -- sorry if it
looked that way!

>> Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
>> --- dh_elpa | 7 +++++-- 1 file changed, 5 insertions(+), 2
>>  deletions(-)
>>
>> diff --git a/dh_elpa b/dh_elpa index 0d3739d..982ac7b 100755 ---
>> a/dh_elpa +++ b/dh_elpa @@ -210,10 +210,11 @@ if ($dh{BYTECOMPILE}) {
>> }
>>  
>>  PACKAGE: -foreach my $package (@{$dh{DOPACKAGES}}) { +foreach my
>> $package (getpackages()) {
>
> I guess it should probably be documented that this is a change needed by
> dh_missing.

What kind of documentation do you need?  the commit message links to
this bug report by number, is associated with the change via "git
blame", and describes dh_missing as the rationale, so that seems like
sufficient documentation to me.  do you want something like local inline
comments?

Pretty much all of the code here is justified by one part of dh or
another, but i don't think the rest of it is commented locally.

Anyway, if you want to tell me what you need, i can produce a variant
patch.  Or, if you want to add inline commentary or whatever other kinds
of documentation you prefer, i'm happy to review it.

Thanks for taking a look at these changes!

       --dkg

Attachment: signature.asc
Description: PGP signature


Reply to: