https://bugzilla.redhat.com/show_bug.cgi?id=1340513 --- Comment #3 from Till Hofmann <hofmann@xxxxxxxxxxxxxxxxxxx> --- Thanks for your update. (In reply to Jerry James from comment #2) > Thank you for the review, Till. > > (In reply to Till Hofmann from comment #1) > > - Large documentation must go in a -doc subpackage. Large could be size > > (~1MB) or number of files. > > Note: Documentation size in /usr/share is 20480 bytes in 4 files, but > > there is > > documentation in /usr/lib/gap/pkg/gbnp/doc with 137 files and size 3.4MB. > > I know from the last review that gap handles documentation differently, > > but is > > it still possible to move those files to a doc package? > > You could also put all doc files in /usr/share and then symlink > > /usr/lib/gap/pkg/gbnp/doc. > > I have put the documentation files into a -doc subpackage That looks good. I think the -doc package should own %{_gap_dir}/pkg/%{pkgname}/ because it doesn't depend on the main package and the directory should be owned even if you only install the doc package, see https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#File_and_Directory_Ownership > > > - There are a lot of files in /usr/lib/gap/pkg/gbnp/doc that look like they > > shouldn't be packaged. Some of them are removed in %install, but all files > > in subfolders are still there. I suggest you replace rm -f in %install with > > some suitable find ... -delete > > I have more aggressively removed some of the files in the doc subdirectory. I would still prefer using find over rm because it's more stable to future changes, but this is okay. > > > - [!]: Uses parallel make %{?_smp_mflags} macro. > > If for some reason you can't use %{?_smp_mflags}, please specify the > > reason in > > the Spec file. > > I added it, but it won't do any good. The only thing being built is the > documentation, and that is an inherently sequential process. I see, but as it doesn't break anything either, I think it's better this way. > > > - There are 81 test files packaged. Do they need to be in the package? If so, > > please specify the reason. > > In addition to having a builtin documentation browser, GAP also has a > builtin package testsuite. For that to function properly, yes, the test > files need to be packaged. Probably very few GAP users run the test suite, > but I want it to work for those that do run it. That makes sense. Can you please add an explanation to the Spec file? This is useful because other users know why those files are packaged, and it allows re-evaluation in the future. > > > - Not sure if the TODO file should be packaged. > > Agreed. I have removed the TODO file. > > > - Please convert the following files to utf8: > > gap-pkg-gbnp.noarch: W: file-not-utf8 /usr/share/doc/gap-pkg-gbnp/TODO > > gap-pkg-gbnp.noarch: W: file-not-utf8 > > /usr/lib/gap/pkg/gbnp/doc/gbnp_doc.tex > > The TODO file is no longer packaged. The other file is a TeX input file, > which tells TeX that it is ISO8859-1, like so: > > \usepackage[latin1]{inputenc} > > Therefore converting the encoding is incorrect in this case. Also, the HTML > output is UTF-8. OK. Why do package tex files in the first place? > > > - spelling: > > gap-pkg-gbnp.noarch: W: spelling-error Summary(en_US) noncommutative -> non > > commutative, non-commutative, noncom mutative > > This should probably be non-commutative. All other spelling errors are > > okay. > > No, "noncommutative" is correct; see > http://www.thefreedictionary.com/noncommutative for example. The problem is > that Fedora's default dictionary contains very few technical terms, which > seems like a terrible idea, given that we use it to check descriptions of > technical products. I have found that the spell checking output of rpmlint > is almost always wrong. I see. I checked Merriam Webster and it didn't know "noncommutative", but if you say that is the correct term, this is fine with me. > > > not necessarily an issue: > > - [?]: Development files must be in a -devel package > > I don't know enough about gap to be able to say which files are development > > files. Please move all development files to a -devel package if there are > > any. > > Hmmm. You know, there really isn't a notion of a development file in GAP, > although perhaps there should be. In this package, the files are used for > runtime (files ending in .g, .gd, or .gi), testing, or documentation. > Perhaps the testing files should be considered development files. I will > give that some thought. Right now, the existing GAP packages bundle the > testing files with the main package, for use by GAP's builtin test suite. I > will do some experiments to see if I can split the testing files out into > separate packages without causing runtime problems. Sounds good. > > New URLs: > Spec URL: https://jjames.fedorapeople.org/gap-pkg-gbnp/gap-pkg-gbnp.spec > SRPM URL: > https://jjames.fedorapeople.org/gap-pkg-gbnp/gap-pkg-gbnp-1.0.3-2.fc25.src. > rpm To summarize, please: - fix ownership of %{_gap_dir}/pkg/%{pkgname}/ in the doc package, - add an explanation about the test files to the Spec file, - clarify why you need to package tex files -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://lists.fedoraproject.org/admin/lists/package-review@xxxxxxxxxxxxxxxxxxxxxxx