https://bugzilla.redhat.com/show_bug.cgi?id=1672047 --- Comment #3 from Antonio Trande <anto.trande@xxxxxxxxx> --- (In reply to Ankur Sinha (FranciscoD) from comment #2) > Looks pretty good, but a few issues with bundling and enabling features. > > Package Review > ============== > > Legend: > [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated > [ ] = Manual review needed > > Issues: > ======= > > - It bundles a lot more things, and I expect we should be able to unbundle a > majority of them that are fortunately already in Fedora: > * Boost (in Fedora) > * Eigen (in Fedora) It's Boost 1.51 (vs Boost 1.66 on Fedora), and eigen3-3.1.1 (vs eigen3-3.3.7 on Fedora) that is not used maybe. Unbundling could be a bad idea. > * zlib (in Fedora) zlib is not used and compilation fails if enable it; probably cannot be unbundled too. > * SFMT (not in Fedora, but looks simple to package) > > - Do the Vcell etc features require Vcell to be packaged already? If not, > that can be enabled. Vcell is on our list, but it's a web app: > https://pagure.io/neuro-sig/NeuroFedora/issue/213 > - VTK support can be enabled. It's in Fedora. Vcell and VTK support are disabled by default and fail if enabled. Need work. > - PDE support can be enabled. I can't find a PDE software, but here's Kairos > which also includes the same Io.cpp and Io.h files that this does: > https://github.com/martinjrobins/Kairos/tree/master/src I wonder if they've > been bundled there too. (It's only two files, so bundling them isn't too > bad) > > - Shouldn't the main package also contain a versioned soname that the binary > will use? Is smoldyn meant to be used as a library---otherwise the devel > package is not needed either. Not needed. It's a private library, not shared. > > - Large documentation must go in a -doc subpackage. Large could be size > (~1MB) or number of files. > Note: Documentation size is 91258880 bytes in 1812 files. > See: > http://fedoraproject.org/wiki/Packaging/Guidelines#PackageDocumentation > ^ All the docs including the examples can go in the docs package. Otherwise > the > devel package becomes very large. Examples are not documentation, they're configuration files for testing. > > - Please do not include a license file if upstream has not provided it. We > should file a bug upstream asking them to do include it. Why not? https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_license_text I will mail to upstream about. > > - Cosmetic issue: could we have all the BRs on individual lines please? It > makes it easier for scripts to work with spec files etc. > > - Please fix rpmlint issues: > smoldyn-devel.x86_64: W: wrong-file-end-of-line-encoding > /usr/share/doc/smoldyn-devel/examples/S2_config/macformat.txt > smoldyn-devel.x86_64: W: wrong-file-end-of-line-encoding > /usr/share/doc/smoldyn-devel/examples/S2_config/windowsformat.txt > smoldyn-devel.x86_64: W: wrong-file-end-of-line-encoding > /usr/share/doc/smoldyn-devel/examples/S94_archive/Andrews_2018/ > note2_transposase/transposase.txt > smoldyn-devel.x86_64: W: wrong-file-end-of-line-encoding > /usr/share/doc/smoldyn-devel/examples/S94_archive/Andrews_2018/note4_MinD/ > MinDdimer.txt > > smoldyn-devel.x86_64: W: spurious-executable-perm > /usr/share/doc/smoldyn-devel/examples/S95_regression/bimolecularlattice.txt > > smoldyn-devel.x86_64: E: wrong-script-end-of-line-encoding > /usr/share/doc/smoldyn-devel/examples/S95_regression/bimolecularlattice.txt > > smoldyn-devel.x86_64: W: spurious-executable-perm > /usr/share/doc/smoldyn-devel/examples/S95_regression/profile3_b.txt > smoldyn-devel.x86_64: W: spurious-executable-perm > /usr/share/doc/smoldyn-devel/examples/S95_regression/profile3_b_nb.txt > smoldyn-devel.x86_64: W: spurious-executable-perm > /usr/share/doc/smoldyn-devel/examples/S95_regression/profile3_e.txt > smoldyn-devel.x86_64: W: spurious-executable-perm > /usr/share/doc/smoldyn-devel/examples/S95_regression/profile3_e_nb.txt > smoldyn-devel.x86_64: W: spurious-executable-perm > /usr/share/doc/smoldyn-devel/examples/S95_regression/profile3_n.txt > smoldyn-devel.x86_64: W: spurious-executable-perm > /usr/share/doc/smoldyn-devel/examples/S95_regression/profile3_n_nb.txt > smoldyn-devel.x86_64: W: spurious-executable-perm > /usr/share/doc/smoldyn-devel/examples/S95_regression/profile3_t.txt > smoldyn-devel.x86_64: W: spurious-executable-perm > /usr/share/doc/smoldyn-devel/examples/S95_regression/profile3_t_nb.txt > smoldyn-devel.x86_64: W: spurious-executable-perm > /usr/share/doc/smoldyn-devel/examples/S95_regression/stick.txt > smoldyn-devel.x86_64: W: spurious-executable-perm > /usr/share/doc/smoldyn-devel/examples/S95_regression/surfacediffuse.txt > > smoldyn-devel.x86_64: W: wrong-file-end-of-line-encoding > /usr/share/doc/smoldyn-devel/examples/S96_bugs/AC1_calcium_lattice.txt > smoldyn-devel.x86_64: W: wrong-file-end-of-line-encoding > /usr/share/doc/smoldyn-devel/examples/S98_utilityprograms/wrl2smol/sample- > surface.wrl > smoldyn-devel.x86_64: W: spurious-executable-perm > /usr/share/doc/smoldyn-devel/examples/S99_more/Min/Min4.txt > smoldyn-devel.x86_64: W: spurious-executable-perm > /usr/share/doc/smoldyn-devel/examples/S99_more/Min/Min5.txt > smoldyn-devel.x86_64: W: wrong-file-end-of-line-encoding > /usr/share/doc/smoldyn-devel/examples/S99_more/antigen/antigen.txt > smoldyn-devel.x86_64: W: wrong-file-end-of-line-encoding > /usr/share/doc/smoldyn-devel/examples/S99_more/li.txt > smoldyn-devel.x86_64: W: spurious-executable-perm > /usr/share/doc/smoldyn-devel/examples/S99_more/microtubule_cata_13_sync_die. > txt > smoldyn-devel.x86_64: E: wrong-script-end-of-line-encoding > /usr/share/doc/smoldyn-devel/examples/S99_more/microtubule_cata_13_sync_die. > txt > smoldyn-doc.noarch: W: spurious-executable-perm > /usr/share/doc/smoldyn-doc/BioNetGen/Faeder_Hlavacek_2009.pdf > -- 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 To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx
