Spec URL: http://mmorsi.fedorapeople.org/staging/rubygem-more_core_extensions.spec SRPM URL: http://mmorsi.fedorapeople.org/staging/rubygem-more_core_extensions-1.1.2-1.fc19.src.rpm Description: A set of core extensions beyond those provided by ActiveSupport. Fedora Account System Username: mmorsi
Hi Mo, this gem still Requires: ruby(abi) = 1.9.1. Would you mind updating it to follow the latest Fedora Ruby Packaging Guidelines?
Sorry for belated response, updated to latest release & to conform to guidelines Spec URL: http://mmorsi.fedorapeople.org/staging/rubygem-more_core_extensions.spec SRPM URL: http://mmorsi.fedorapeople.org/staging/rubygem-more_core_extensions-1.2.0-1.fc20.src.rpm Koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=7436745
Hi Mo, Please accept my apologies at taking so long to get back on this. Here's my review. Package is APPROVED. The following comments are not blockers, but suggestions: - You can strip out the boilerplate gem2rpm comment about more_core_extensions-1.2.0.gem at the top of the file, since this is going to go stale. - rpmlint doesn't like the length of the Summary field. For some reason I have a vague memory that the Summary field is not supposed to contain the name of the package... but I can't find the guideline for that at the moment. At any rate, you could shorten the Summary to simply "Set of core extensions beyond those provided by ActiveSupport". That should fix the "summary-to-long" rpmlint error. (This could be fixed in the gemspec upstream as well :) - No need to BuildRequires: ruby when you've already got BuildRequires: ruby(release). BR: ruby is only for gems that can only work on MRI. - The coveralls and rspec BRs are commented out, and I'm not sure why they are commented. It seems like you should be able to run the test suite, right? The %check section is blank (it's just the default boilerplate from gem2rpm). - With the changes in the Fedora 21 Ruby Packaging guidelines, you don't have to provide explicit Requires or Provides any more for rubygem packages. For backwards compatibility with Fedora 20 and RHEL 7, you can wrap your Requires and Provides like so: %if 0%{?fc19} || 0%{?fc20} || 0%{?el7} Requires: ruby(release) Requires: ruby(rubygems) Requires: rubygem(activesupport) > 3.2 %endif %if 0%{?fc19} || 0%{?fc20} || 0%{?el7} Provides: rubygem(%{gem_name}) = %{version} %endif And then just strip out each dist conditional as Fedora 19 goes EOL, then Fedora 20, etc. - The %license macro has not yet been adopted as a requirement by the Packaging Committee, but you can start to use it now, with a backwards-compatible shim, like so: %files %{!?_licensedir:%global license %%doc} %dir %{gem_instdir} %license %{gem_instdir}/LICENSE.txt - In the %changelog section, your name is listed as "mmorsi", and this should be "Mo Morsi". (You might want to check the output of the rpmdev-packager utility on your box to be sure this is doing the Right Thing.) - The %{gem_instdir}/spec directory should not be marked as "%doc" since the code there is not documentation. Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed ===== MUST items ===== Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: License field in the package spec file matches the actual license. [x]: License file installed when any subpackage combination is installed. [x]: Package must own all directories that it creates. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [-]: Development files must be in a -devel package [?]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: Package is not known to require an ExcludeArch tag. [x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc. [x]: Package requires other packages for directories it uses. [x]: Package does not own files or directories owned by other packages. [x]: All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package do not use a name that already exist [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. [x]: Packages must not store files under /srv, /opt or /usr/local Ruby: [x]: Platform dependent files must all go under %{gem_extdir_mri}, platform independent under %{gem_dir}. [x]: Gem package must not define a non-gem subpackage [x]: Macro %{gem_extdir} is deprecated. [x]: Gem package is named rubygem-%{gem_name} [x]: Package contains BuildRequires: rubygems-devel. [x]: Gem package must define %{gem_name} macro. [x]: Pure Ruby package must be built as noarch [x]: Package does not contain Requires: ruby(abi). [x]: Package contains Requires: ruby(release). ===== SHOULD items ===== Generic: [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: Avoid bundling fonts in non-fonts packages. [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [!]: Final provides and requires are sane (see above). [x]: Fully versioned dependency in subpackages if applicable. [?]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [x]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x]: Package should compile and build into binary rpms on all supported architectures. [!]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Sources can be downloaded from URI in Source: tag [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: Dist tag is present (not strictly required in GL). [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified. Ruby: [x]: Gem should use %gem_install macro. [x]: Specfile should use macros from rubygem-devel package. [x]: Gem package should exclude cached Gem. [x]: Test suite should not be run by rake. [x]: Test suite of the library should be run. ===== EXTRA items ===== Generic: [!]: Spec file according to URL is the same as in SRPM. Note: Spec file as given by url is not the same as in SRPM (see attached diff). See: (this test has no URL) [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). Rpmlint ------- Checking: rubygem-more_core_extensions-1.2.0-1.fc22.noarch.rpm rubygem-more_core_extensions-doc-1.2.0-1.fc22.noarch.rpm rubygem-more_core_extensions-1.2.0-1.fc22.src.rpm rubygem-more_core_extensions.noarch: E: summary-too-long C MoreCoreExtensions are a set of core extensions beyond those provided by ActiveSupport rubygem-more_core_extensions.noarch: E: useless-provides rubygem(more_core_extensions) rubygem-more_core_extensions.src: E: summary-too-long C MoreCoreExtensions are a set of core extensions beyond those provided by ActiveSupport 3 packages and 0 specfiles checked; 3 errors, 0 warnings. Rpmlint (installed packages) ---------------------------- # rpmlint rubygem-more_core_extensions rubygem-more_core_extensions-doc rubygem-more_core_extensions.noarch: E: summary-too-long C MoreCoreExtensions are a set of core extensions beyond those provided by ActiveSupport rubygem-more_core_extensions.noarch: E: useless-provides rubygem(more_core_extensions) 2 packages and 0 specfiles checked; 2 errors, 0 warnings. Diff spec file in url and in SRPM --------------------------------- --- /home/ktdreyer/fedora-scm/rubygem-more_core_extensions/1052283-rubygem-more_core_extensions/srpm/rubygem-more_core_extensions.spec 2014-12-06 13:16:26.471084878 -0700 +++ /home/ktdreyer/fedora-scm/rubygem-more_core_extensions/1052283-rubygem-more_core_extensions/srpm-unpacked/rubygem-more_core_extensions.spec 2014-08-21 13:10:05.000000000 -0600 @@ -78,17 +78,4 @@ %changelog -* Thu Aug 21 2014 Mo Morsi <mmorsi> - 1.2.0-1 -- Update to latest Fedora guidelines - -* Fri Aug 23 2013 Mo Morsi <mmorsi> - 1.1.2-1 -- Release 1.1.2 - -* Fri Aug 23 2013 Mo Morsi <mmorsi> - 1.0.2-1 -- Bumped version - -* Fri Aug 23 2013 Mo Morsi <mmorsi> - 1.0.1-1 -- Bumped version - -* Thu Jun 20 2013 Steve Linabery <slinaber> - 1.0.0-1 +* Thu Aug 21 2014 mmorsi <mmorsi> - 1.2.0-1 - Initial package - Requires -------- rubygem-more_core_extensions (rpmlib, GLIBC filtered): ruby(release) ruby(rubygems) rubygem(activesupport) rubygem-more_core_extensions-doc (rpmlib, GLIBC filtered): rubygem-more_core_extensions Provides -------- rubygem-more_core_extensions: rubygem(more_core_extensions) rubygem-more_core_extensions rubygem-more_core_extensions-doc: rubygem-more_core_extensions-doc Source checksums ---------------- https://rubygems.org/gems/more_core_extensions-1.2.0.gem : CHECKSUM(SHA256) this package : 354b29d72ab85ad6603c59474d7edb05c8870b413cdad79b66f1fe940b1fabc4 CHECKSUM(SHA256) upstream package : 354b29d72ab85ad6603c59474d7edb05c8870b413cdad79b66f1fe940b1fabc4 Generated by fedora-review 0.5.2 (63c24cb) last change: 2014-07-14 Command line :/usr/bin/fedora-review -m fedora-rawhide-x86_64 -b 1052283 Buildroot used: fedora-rawhide-x86_64 Active plugins: Generic, Ruby, Shell-api Disabled plugins: Java, C/C++, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP Disabled flags: EXARCH, EPEL5, BATCH, DISTTAG
https://github.com/ManageIQ/more_core_extensions/blob/master/spec/spec_helper.rb#L79-L83 begin require 'coveralls' Coveralls.wear! rescue LoadError end So coveralls is optional! Good on you guys :) I've been trying to get that exact change into more gems upstream.
Thanks. Final updated package: Spec: https://mmorsi.fedorapeople.org/staging/rubygem-more_core_extensions.spec SRPM: https://mmorsi.fedorapeople.org/staging/rubygem-more_core_extensions-1.2.0-2.fc20.src.rpm Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=8332003 New Package SCM Request ======================= Package Name: rubygem-more_core_extensions Short Description: Set of core extensions beyond those provided by ActiveSupport Upstream URL: https://github.com/ManageIQ/more_core_extensions Owners: mmorsi Branches: f21 f20 InitialCC: (In reply to Ken Dreyer from comment #3) > Hi Mo, > > Please accept my apologies at taking so long to get back on this. Here's my > review. Package is APPROVED. > > The following comments are not blockers, but suggestions: > > - You can strip out the boilerplate gem2rpm comment about > more_core_extensions-1.2.0.gem at the top of the file, since this is going > to go stale. Done > > - rpmlint doesn't like the length of the Summary field. For some reason I > have a vague memory that the Summary field is not supposed to contain the > name of the package... but I can't find the guideline for that at the > moment. At any rate, you could shorten the Summary to simply "Set of core > extensions beyond those provided by ActiveSupport". That should fix the > "summary-to-long" rpmlint error. (This could be fixed in the gemspec > upstream as well :) Done > > - No need to BuildRequires: ruby when you've already got BuildRequires: > ruby(release). BR: ruby is only for gems that can only work on MRI. > Done > - The coveralls and rspec BRs are commented out, and I'm not sure why they > are commented. It seems like you should be able to run the test suite, > right? The %check section is blank (it's just the default boilerplate from > gem2rpm). > There are a few broken specs reported upstream and seems to be a few more when run against the latest gem stack in rawhide. Leaving these as ommitted for the time being until the situation can be resolved. > - With the changes in the Fedora 21 Ruby Packaging guidelines, you don't > have to provide explicit Requires or Provides any more for rubygem packages. > For backwards compatibility with Fedora 20 and RHEL 7, you can wrap your > Requires and Provides like so: > > %if 0%{?fc19} || 0%{?fc20} || 0%{?el7} > Requires: ruby(release) > Requires: ruby(rubygems) > Requires: rubygem(activesupport) > 3.2 > %endif > > %if 0%{?fc19} || 0%{?fc20} || 0%{?el7} > Provides: rubygem(%{gem_name}) = %{version} > %endif > > And then just strip out each dist conditional as Fedora 19 goes EOL, then > Fedora 20, etc. Done > > - The %license macro has not yet been adopted as a requirement by the > Packaging Committee, but you can start to use it now, with a > backwards-compatible shim, like so: > > %files > %{!?_licensedir:%global license %%doc} > %dir %{gem_instdir} > %license %{gem_instdir}/LICENSE.txt Done > > - In the %changelog section, your name is listed as "mmorsi", and this > should be "Mo Morsi". (You might want to check the output of the > rpmdev-packager utility on your box to be sure this is doing the Right > Thing.) Seems to be correct. > > - The %{gem_instdir}/spec directory should not be marked as "%doc" since the > code there is not documentation. Done. Again thx for review.
Git done (by process-git-requests).
This package was approved and imported in repositories, but this review ticket was never closed. I'm closing it now.