Hello Sean, Sorry for being late. I was having a holiday and turned to something else in the past few days :p
2016-09-22 10:33 GMT+08:00 Sean Whitton <spwhit...@spwhitton.name>: > Hello Boyuan, > > A few new things: > > 1. You dropped --parallel from d/rules without explaining why in the commit > message. Does it break the build? Certainly not essential, but it's > nice to enable it since our buildds are so overworked. --parallel is no longer needed to build in parallel since debhelper v10. I hope the build machines could recognize new debhelper soon. > 2. Some grammatical errors in package descriptions: > > s/documentations/documentation/ > > s/on Evernote site/on the Evernote site/ done. > Since 'complete' is an extreme adjective, it is odd to qualify it as > 'rather complete'. I would s/rather //. Sorry but I am not intended to fix (?) it. The function is not complete due to new releases of Evernote Cloud API recently, and such usages can be found all around the Internet. > 3. I observed this: > > hephaestus ~ % objdump -p /usr/bin/nixnote2 | grep NEEDED > NEEDED libhunspell-1.4.so.0 > NEEDED libcurl.so.4 > NEEDED libpoppler-qt5.so.1 > NEEDED libqt5qevercloud.so.3 > NEEDED libQt5PrintSupport.so.5 > NEEDED libQt5WebKitWidgets.so.5 > [...] > > Maybe the SONAME of qevercloud should be libQt5qevercloud, not > libqt5evercloud, to match this convention? Since we can't change this > in the future, it would be good to get it right now. Maybe this is > documented somewhere... Upstream uses this name intentionally (the name is written in CMakeLists.txt). I guess he did not want to let his third-party library get confused with official Qt5 libraries. > Your `dch -r` is behind HEAD again, and your debian/3.0.2+ds-1 isn't on > the HEAD of master. OK now I'm trying to run dch -r on every commit. :) >> I dropped the tex / pdf / postscript files in the -doc package because >> they are not that >> useful when html files are provided as well. > > Although HTML is considered the primary format for documentation in > Debian packages, I would suggest including the PDFs and PS files anyway. > Someone might want to print the documentation, or they might just prefer > reading PDFs. Since it's in a separate -doc package, we don't have to > worry about cluttering up anyone's system. > > If you agree, and install the PDFs and PSs, it would also be a good idea > to move the html docs into /usr/share/doc/qevercloud-doc/html. That sounds good even if PDFs are not to be installed. I am putting them inside the sub-directory html. > Whether or not you build the PDFs, it would be good to handle this > error, which could be worrying to someone: > > sh: 1: epstopdf: not found > error: Problems running epstopdf. Check your TeX installation! > > If you don't want to install the PDFs, maybe you can instruct doxygen > not to try to run epstopdf, so that the error is not emitted. I tried to build PDFs, but the dependency is just too large (a lot of texlive packages). What's more, I met FTBFS with current texlive. Bug forwarded to https://github.com/d1vanov/QEverCloud/issues/8. Added a patch to disable the instructions about PDFs in Doxyfile. > Where you have > > dh_auto_{build,install} > dh_auto_{build,install} --builddirectory=$(_QEVERCLOUD_QT5_BUILDDIR) > > it's not clear why you have to call it twice. I suggest adding a > comment saying what each of the two calls does. Some more comments are added. >> > 11. Upstream's README.md seems like a useful file. Consider installing >> > it to /usr/share/doc in both your -dev packages. You should patch it to >> > remove the stuff about building and installing the library, though. >> >> I thought dh_installdocs would install it (as stated in the man page) but it >> didn't. Wrote it into docs file explicitly and patched now. > > I've reported #838538. > >> The Git repository on Github has been update, and the new packages are >> uploaded to mentors and debomatic-amd64. The binary packages on debomatic >> suggests that nixnote2 successfully recognized the libqt5qevercloud3 >> shlib version >> requirement. > > I did some test builds and everything is looking good :) > > However, I did see this output: > > dpkg-shlibdeps: warning: package could avoid a useless dependency if > debian/libqt4qevercloud3/usr/lib/i386-linux-gnu/libqt4qevercloud.so.3 > was not linked against libdl.so.2 (it uses none of the library's > symbols) > dpkg-shlibdeps: warning: package could avoid a useless dependency if > debian/libqt5qevercloud3/usr/lib/i386-linux-gnu/libqt5qevercloud.so.3 > was not linked against libQt5Gui.so.5 (it uses none of the library's > symbols) > dpkg-shlibdeps: warning: package could avoid a useless dependency if > debian/libqt5qevercloud3/usr/lib/i386-linux-gnu/libqt5qevercloud.so.3 > was not linked against libdl.so.2 (it uses none of the library's > symbols) > > Do you know whether those unneeded dependencies may be avoided? Writing "export DEB_LDFLAGS_MAINT_APPEND = -Wl,--as-needed" in debian/rules to clean the unneeded links. libQt5Gui is gone now but libdl still exists. Well they look harmless and I would just leave them there. > One more thing -- the .so should be installed as > libqt?evercloud.so.3.0.0, with a symlink from libqt?evercloud.so.3. See > ch. 8 of Debian policy for an explanation of this practice. Patch added (0010-patch-library-soname-chain.patch). Issue forwarded: https://github.com/d1vanov/QEverCloud/issues/7 > > Thanks! I'm excited to see the completion of this RFS! > > -- > Sean Whitton Thanks. The GitHub repo / debian-mentors / debomatic-amd64 files are update-to-date now. Sincerely, Boyuan Yang