Hi Steven, Hi Gert, Steven Robbins, on 2020-08-08 14:22:08 -0500: > I've looked through all the changes now. I have some questions. > > Inline ITK data v4.13.3 > 7c3eb773ef1a698af32360ecd020bab997aed584 > > It looks like you unpacked the data into data/ in the upstream branch. > However, the upstream/4.13.3-dfsg1 tag is prior to this commit, so the > orig.tar generation does NOT include the data. It seems to me that we should > take one of two approaches: > > 1. Retain the upstream "source" and "data" tarballs, in which case the above > commit should be reverted so as to keep the upstream branch consistent with > the upstream "source-only" tarball. > > 2. Merge the data and source. But then the source would be named using the > tag on the above commit (i.e. upstream/4.13.3-dfsg1+data) > > The first approach was used previously -- which is why there are two "orig" > tarballs -- but the second has merit also in that we have only a single, > bigger, tarball. Opinions welcome -- I'm ok with either approach. Looking back to the upstream branch, if I understand correctly, previous upstream/$vers-dfsg1 tags are pointing to a directory with both source and data, which seems more consistent with option 2, except the needed tag is in use with the source only variant. My impression is option 2 would be fine to resolve the present situation; but I don't have much experience with resolving conflicts with VCSs, so take it with a grain of salt. Sorry about the rather shaky situation with the upgrade. :/ Lintian left an informational notice about the get-orig-source target, so there is probably still something to think for the next versions. I read about multiple tarballs support in uscan, that came with version=4, but there have already been discussions here on this topic, and it seems to not be a very welcome option; but maybe it's better for existing packages ? One other approach could be to have a "source" package storing ITK data only, on which the insighttoolkit source would have a build dependency ? But I guess this new package will have to go through the NEW queue. > updated d/patches/remove_gcc_version_test.patch > 36497a6d100b0c33c72072c0db7104b2c3f05e6d > > This is a question more for Gert, but it doesn't seem wise to me to remove the > #define VCL_GCC from vcl_compiler.h as this symbol is tested in a few places. > Nor does it seem wise to pretend to be GCC 7.3 (by defining VCL_GCC_73). > Rather, why not change the patch to do the following: > > (a) change error "Dunno about this gcc" to a warning > (b) patch out test_preprocessor.cxx entirely; it is brittle and not a very > useful test anyway I must admit I have merely ported the patch to apply against the new version, but haven't dug further into it. > d/patches/series: mention unused patch as comment > 18a91242b65b570d98a2099f4a6d630a43fd0f97 > > This patch was removed from series previously saying it was incorporated > upstream. I will just delete the patch and the comment from d/p/series. This sounds reasonable if it is not needed anymore. > Rest of the commits look solid. Once we iron out the above issues, I would be > comfortable in doing the upload. > > -Steve Many thanks for your time and your review! :) Kind Regards, -- Étienne Mollier <etienne.mollier@mailoo.org> Old rsa/3072: 5ab1 4edf 63bb ccff 8b54 2fa9 59da 56fe fff3 882d New rsa/4096: 8f91 b227 c7d6 f2b1 948c 8236 793c f67e 8f0d 11da Sent from /dev/pts/6, please excuse my verbosity.
Attachment:
signature.asc
Description: PGP signature