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

Re: Bug#909716: Request for sponsoring changes in idba



Hi Andreas, 

On Tue, 10 Mar, 2020, 9:21 PM Andreas Tille, <andreas@an3as.eu> wrote:

Fine.  That works.  Sorry when I have given a bit short-cutted advise.
I've completed the long description.

Oh sorry I missed. Actually I changed it at first but it didn't work so I went back and forth a few times and forgot to complete it. 

In my commit 6cff0c2377861eb179d4ec06514df75df1323510 I tried to fix this
but did not test which I'll leave you for an exercise. ;-)

Thanks, this worked, but initially it didn't compile. The error was that it couldn't find any *.1 files in debian/idba and debian/idba-extra.
However, when I renamed the two folders to idba-man and idba-extra-man, it suddenly worked. Do you have any ideas why this was so?
 
When doing so I stumbled upon the script run-unittest.py which I think
should not end up in /usr/bin.  I installed it into docs and your second
exercise could be to check whether it really does some sensible testing
at all.  As far as I can see it is just triggering the build time test
but it does not look as if it be useful to install on users machines.
Even if it would make sense the name /usr/bin/run-unittest.py is way to
generic and should be avoided.  Same with /usr/bin/scan.py - please
check whether this script makes sense in /usr/bin it should be probably
renamed (to something like idba_scan.py).  Otherwise it should also go
to /usr/share/doc/idba (or left out fully).

Yes, it should definitely not go to debian/bin, sorry.
Line 17 in debian/rules [1] says, "for the moment the role of these scripts is totally unknown but they do not seem to be necessary," and proceeds to copy run-unittest.py, scan.py, validate_blat and validate_blat_parallel to usr/lib/idba.
I tried commenting out these lines, and the package seems to work just fine. They seem to be required only during compilation and testing, so they probably shouldn't be installed as binaries in the final user installation. Maybe we could let them be in the lib folder? 
I'm not sure, however. Please take a look and let me know if I should change it to docs.
 
> I have also changed the test files now. Only a few necessary files are
> included, and the rest are generated during testing.

That's really good!  The only think I'm missing now is that we should
always mention the origin where the data were obtained from (may be in
a script calling wget for downloading).  It makes also sense to drop
some note about the license of these files.

I have added a file called data-source.sh in debian/tests/test-data. Is this the right way to do it?
Also, I obtained the data from their research papers about these tools. What kind of license would be applicable in this case?
And where do I get the license from?
 
> Let me know if I missed anything, otherwise please review and sponsor these
> changes.

What you did is basically correct and extremely helpful to finalise the
autopkgtest.  Please inspect my recent commits and try to provide the
missings I mentioned above in this mail.

I added a few examples by extending the debian/createmanpages script.
I'd perfectly accept some kind of "lazyness" here.  I would not mind
uploading the current incomplete status despite of the lintian warnings.
I usually decide according to the "importance" of the tool that needs a
manpage.  My motivation was to get "the first three in list" and also
the ones we are using in autopkgtest.  Feel free to decide whether you
want a complete set of manpages or not.

Okay. I think the set of manpages we have should be enough, given that the other tools aren't too important.

Thanks for the intro to dh_install, man pages and documentation about packages. 
Please let me know if any more work is required on this package.

Regards,
Pranav

Reply to: