El 16/08/16 a las 09:15, Gianfranco Costamagna escribió: > control: tags -1 moreinf > > Hi, > >> * Initial release (Closes: #739035) > lets try a review:
Hi gianfranco! lets go: > > 1) std-version is 3.9.8 now fixed. > > 2) debhelper (>= 9), libncurses5-dev (>= 5.9), > unzip (>= 6.0), libsdl2-dev (>= 2.0.2), libsdl1.2-dev (>= 1.2.15), > gcc (>= 4:4.9) > > > do you really need both sdl1.2 and sdl2? > do you really need the version constraints for each dependency? > why gcc is listed here? > please drop versions when already satisfied in jessie, or wheezy in case you > want to try > a backport-sloppy (I would really avoid that) remove gcc, let sdl 1.2 (remove unused sdl2) update versions from unstable fixed. > > 3) > Architecture: i386 amd64 > Depends: ${shlibs:Depends}, ${misc:Depends}, libncurses5 (>= 5.9), > libsdl2-2.0-0 (>= 2.0.2), libsdl1.2debian (>= 1.2.15) >why only two architectures? why aren't the runtime dependencies picked up with >shlibs:Depends? > ldd debian/syncterm/usr/bin/syncterm > linux-vdso.so.1 => (0x00007ffeca745000) > libutil.so.1 => /lib/x86_64-linux-gnu/libutil.so.1 (0x00007efc98d79000) > libncurses.so.5 => /lib/x86_64-linux-gnu/libncurses.so.5 > (0x00007efc98b57000) > libtinfo.so.5 => /lib/x86_64-linux-gnu/libtinfo.so.5 > (0x00007efc9892d000) > libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007efc98729000) > libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007efc98420000) > libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 > (0x00007efc98202000) > libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007efc97e39000) > /lib64/ld-linux-x86-64.so.2 (0x0000560b91261000) > > > at least they seems to be mostly not linked at runtime. fixed. i only can test in these archs, but i changed it to => any > > 4) rules: to understand the platform you are building, I suggest to use > dpkg-architecture > dpkg-architecture -qDEB_TARGET_* > im cleanup the code and use dpkg-architecture now. Great! fixed > 5) override_dh_strip: > dh_strip --dbg-package=syncterm-dbg > > > please avoid dbg packages, they are auto generated now true! i remove it. fixed > > > 6) ls src/ > build comio conio sbbs3 smblib syncterm uifc xpdev > > some (most) of them looks like embedded libraries it's part of uptream source. can i do something to make it better? > > 7) disabled_cryptlib needs to end in .patch (also in series file you should > change it) fixed. > --- > The information above should follow the Patch Tagging Guidelines, please > checkout http://dep.debian.net/deps/dep3/ to learn about the format. Here > are templates for supplementary fields that you might want to add: > > this seems useless fixed too > > 8) the license is non dfsg > > The files sbbs3/zmodem.h and sbbs3/zmodem.c are derived from the > zmtx/zmrx package available at > ftp://ftp.netsw.org/net/modem/protocols/zmodem/zmtx-zmrx/ > . > The licence contained in the archive is: > . > MCS allows you to use and copy/modify this source under the following > conditions: > . > - MCS or Jacques Mattheij shall not be liable for any damages arising > from the use of this code > - the archive must be distributed as a whole leaving version numbers intact. > please do not distribute modifications; mail them back to us for inclusion > in the next release which should follow each other fairly quickly in > the beginning > - you will not use this software for commercial purposes. > (commercial licenses are available contact us for info) > . > As such, this program may not be redistributable. YOU HAVE BEEN WARNED! > . > If anyone can put me (sh...@sasktel.net) in contact with the authours, > that would be greatly appreciated. > this was fixed. https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=777651#57 You can check the zmodem.* header file on cvs http://cvs.synchro.net/cgi-bin/viewcvs.cgi/src/sbbs3/zmodem.h?revision=1.54&view=markup Unfortunately, this was after 1.0 release date, and it was made through my work to contact everyone involved and good response from them. > > 9) LGPL with no versioning is wrong. fixed > > 10) many missing licenses: e.g. BSD-4-clause > 11) many missing copyrights, e.g. > grep copyright . -Ri > > stopping here the review, because of 8, that needs to be fixed upstream I > think > I think that can parse file by file to fine copyright settings. i update it, please check, I want to the package fit the debian legal requeriments, the code is gpl, lgpl and some files bsd. i think that all are dfsg The non-dfsg part was cryplib and was removed via patch (the app can be used without that lib) > automatic checks from check-all-the-things: > > $ env PERL5OPT=-m-lib=. cme check dpkg > [lots] > $ codespell --quiet-level=3 > [lots] > $ cppcheck -j1 --quiet -f . > [lots] > $ find -type f -iname '*.desktop' -exec desktop-file-validate {} \; > [some] > $ fdupes -q -r . | grep -vE > '/(\.(git|svn|bzr|hg|sgdrawer)|_(darcs|FOSSIL_)|CVS)(/|$)' | cat -s > [lots] > $ flawfinder -Q -c . > [some] > > and so on > $ suspicious-source > ./src/uifc/uifc.c check-all-the-things are cool tool! I did not know I will use now it for every package! i upload again to mentors: https://mentors.debian.net/debian/pool/main/s/syncterm/syncterm_1.0+dfsg-1.dsc thanks, I really very apprecited your review. -- Fernando Toledo Dock Sud BBS http://bbs.docksud.com.ar telnet://bbs.docksud.com.ar