https://bugzilla.redhat.com/show_bug.cgi?id=1631988 --- Comment #2 from Neal Gompa <ngompa13@xxxxxxxxx> --- Spec review: > Release: %{_release_simple}%{?dist} You should just put the number in. That is, it should be "2%{?dist}". Strictly speaking, it's not required to change this, and if you have particular automation centered around that, you should document it. Cf. https://fedoraproject.org/wiki/Packaging:DistTag > License: GPLv2 A quick glance at the source files for the project indicate that this is actually GPLv2+. Please adjust accordingly. > Source1: https://github.com/juliogonzalez/s3fs-fuse-rpm/blob/%{version}-%{_release_simple}/SOURCES/passwd-s3fs It's better to change this to a plain source reference. When the package is imported into Dist-Git, the file will exist in the Dist-Git repository anyway. > Requires: fuse >= 2.8.4 > Requires: fuse-libs >= 2.8.4 Is there a reason for this? Can't we just depend on the autodeps here? Or do we also need the fuse CLI tools, too? > Requires: curl >= 7.0 > Requires: libxml2 >= 2.6 > Requires: openssl >= 0.9 Why do these need to be explicitly requested? > BuildRequires: curl-devel, fuse-devel, libxml2-devel, openssl-devel > BuildRequires: automake, gcc-c++, make Please lay these out with one BR per line, as it makes it easier for diffing. Since the software build code uses pkgconfig modules, please use `pkgconfig()` deps for these where possible. > Conflicts: fuse-s3fs Please put a comment in the spec about why there's a conflict here. > Obsoletes: s3fs Unversioned Obsoletes are not generally allowed in Fedora, and I don't believe we have an "s3fs" to replace. Cf. https://fedoraproject.org/wiki/Packaging:Guidelines#Renaming.2FReplacing_Existing_Packages > %description > s3fs is a FUSE file system that allows you to mount an Amazon S3 bucket as a > local file system. It stores files natively and transparently in S3 (i.e., > you can use other programs to access the same files). Maximum file size=64GB > (limited by s3fs, not Amazon). > . > s3fs is stable and is being used in number of production environments, e.g., > rsync backup to s3. Please drop the line with only a ".", as that is not required for newline separation in spec files. > %global debug_package %{nil} Please drop this, as there's no reason that debuginfo cannot be produced for this package. > %setup -q "%autosetup" is preferable to "%setup -q". If you're concerned about it not working for EPEL builds, that's not a problem, as %autosetup has been backported to all supported EPEL targets. Cf. http://rpm.org/user_doc/autosetup.html > make %{?_smp_mflags} "%make_build" is preferred over "make %{?_smp_mflags}". Like "%autosetup", this has been backported to all supported EPEL targets. > make install DESTDIR=%{buildroot} Please use "%make_install". > cp -p %{SOURCE1} passwd-s3fs This belongs in the prep section. > %doc AUTHORS README.md ChangeLog COPYING passwd-s3fs "%license" must be used for COPYING in the file list. Again, this works on all targets. Cf. https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text -- 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
