[Bug 1121082] Review Request: rubygem-clockwork - A scheduler process to replace cron
https://bugzilla.redhat.com/show_bug.cgi?id=1121082 František Dvořák changed: What|Removed |Added CC||val...@civ.zcu.cz Assignee|nob...@fedoraproject.org|val...@civ.zcu.cz Flags||fedora-review? --- Comment #1 from František Dvořák --- Taking the review... Can we do review swap with #1131991 (rubygem-logstash-event)? -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1121082] Review Request: rubygem-clockwork - A scheduler process to replace cron
https://bugzilla.redhat.com/show_bug.cgi?id=1121082 --- Comment #2 from František Dvořák --- 1) missing license text (some MIT variants even require to distribute it with the sources), upstream needs to be notified; the good thing is the README links to exact MIT variant [http://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text] (This week I bumped to the same problem with logstasher :-), I'll submit it for review later, there were other issues yet...) 2) missing man-pages: packagers should work with upstream to add them, but it is not strictly required by guidelines [https://fedoraproject.org/wiki/Packaging:Guidelines?rd=PackagingGuidelines#Man_pages] I wrote simple man-page for clockworkd (with texts from help a README), it could be used (and maybe improved): http://scientific.zcu.cz/fedora/REVIEWS/clockworkd.1 3) you could prepare commented out exact steps in %check, for minitest I have seen this magic formula in the ruby list: ruby -Ilib -e 'Dir.glob "./test/**/*_test.rb", &method(:require)' I was able to launch the tests this way, after installing dependencies and 'gem install contests'. 4) cosmetic: timestamp of the source gem in .src.rpm should be rather 2014-07-05 curl and wget supports setting of "remote" timestamp, but it is not enabled by default [http://fedoraproject.org/wiki/Packaging:Guidelines#Timestamps] -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1121082] Review Request: rubygem-clockwork - A scheduler process to replace cron
https://bugzilla.redhat.com/show_bug.cgi?id=1121082 --- Comment #3 from Josef Stribny --- > 1) missing license text (some MIT variants even require to distribute it with > > the sources), upstream needs to be notified; the good thing is the README > links to exact MIT variant True, I asked the upstream to include it[1]. > 2) missing man-pages: packagers should work with upstream to add them, but it > is not strictly required by guidelines Yes, that would be nice. Do you have the source file for the man page? For example I used asciidoc format in sdoc man pages. Or did you really wrote this man page as it is? I would rather submit the source to upstream. Nevertheless, I included your man page in the spec file. Depending on your answer I will submit it to upstream and link the issue. >3) you could prepare commented out exact steps in %check, for minitest I have > seen this magic formula in the ruby list: Well, you are combing RPM packaged gems with upstream ones. Of course you can also run only upstream test suite with every gem which is why I don't see the reason to put it there. When we have everything in Fedora, I would add the proper check section. 4) cosmetic: timestamp of the source gem in .src.rpm should be rather 2014-07-05 I admit that I don't really care about this much. Would that affect Fedora users somehow? Most of us download .gem files from RubyGems.org by `gem fetch` command. Spec URL: http://data-strzibny.rhcloud.com/obs/rubygem-clockwork.spec SRPM URL: http://data-strzibny.rhcloud.com/obs/rubygem-clockwork-0.7.7-2.fc22.src.rpm [1] https://github.com/tomykaira/clockwork/issues/116 -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1121082] Review Request: rubygem-clockwork - A scheduler process to replace cron
https://bugzilla.redhat.com/show_bug.cgi?id=1121082 --- Comment #4 from František Dvořák --- (In reply to Josef Stribny from comment #3) > > 2) missing man-pages: packagers should work with upstream to add them, but > > it > > is not strictly required by guidelines > > Yes, that would be nice. Do you have the source file for the man page? For > example I used asciidoc format in sdoc man pages. Or did you really wrote > this man page as it is? I would rather submit the source to upstream. > > Nevertheless, I included your man page in the spec file. Depending on your > answer I will submit it to upstream and link the issue. > Yes, I wrote it directly. Of course using some preferred ruby way instead would be also possible improvement. > >3) you could prepare commented out exact steps in %check, for minitest I have > > seen this magic formula in the ruby list: > > Well, you are combing RPM packaged gems with upstream ones. Of course you > can also run only upstream test suite with every gem which is why I don't > see the reason to put it there. When we have everything in Fedora, I would > add the proper check section. > Yes, I liked the idea have to have it ready in .spec just to uncomment, but that's really up to package maintainer. :-) > 4) cosmetic: timestamp of the source gem in .src.rpm should be rather > 2014-07-05 > > I admit that I don't really care about this much. Would that affect Fedora > users somehow? Most of us download .gem files from RubyGems.org by `gem > fetch` command. > Right, that's just cosmetic and I don't think it will affect anything. -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1121082] Review Request: rubygem-clockwork - A scheduler process to replace cron
https://bugzilla.redhat.com/show_bug.cgi?id=1121082 --- Comment #5 from Josef Stribny --- Ok, I asked upstream to include it[1]. I would include this link above the source in the spec file before pushing. Anything else? [1] https://github.com/tomykaira/clockwork/pull/117 -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1121082] Review Request: rubygem-clockwork - A scheduler process to replace cron
https://bugzilla.redhat.com/show_bug.cgi?id=1121082 --- Comment #6 from František Dvořák --- 1) there is still a problem with the license: In this case we should also include local copy of the license text, because this MIT variant requires it: "The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software." The most important work of contacting upstream is done, though... 2) you can consider removing "ruby(release)" and "ruby" BuildRequires (rubygems already picks it) And could you review https://bugzilla.redhat.com/show_bug.cgi?id=1131991 (rubygem-logstash-event)? :-) -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1121082] Review Request: rubygem-clockwork - A scheduler process to replace cron
https://bugzilla.redhat.com/show_bug.cgi?id=1121082 --- Comment #7 from Josef Stribny --- > 1) there is still a problem with the license: I have done what I had to: ask upstream to include it. I didn't create the file in the end at they link to a template that they should fill in (author & year) and I am not sure what to put there. > 2) you can consider removing "ruby(release)" and "ruby" BuildRequires > (rubygems already picks it) Yes, it's redundant, I will remove it. -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1121082] Review Request: rubygem-clockwork - A scheduler process to replace cron
https://bugzilla.redhat.com/show_bug.cgi?id=1121082 --- Comment #8 from Josef Stribny --- I am sorry, it was just added by upstream[0]. I am going to include it and submit an updated srpm. [0] https://github.com/tomykaira/clockwork/commit/13e78ddb31413fdbc26028577d5fcfbb6913714f -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1121082] Review Request: rubygem-clockwork - A scheduler process to replace cron
https://bugzilla.redhat.com/show_bug.cgi?id=1121082 --- Comment #9 from Josef Stribny --- Done: Spec URL: http://data-strzibny.rhcloud.com/obs/rubygem-clockwork.spec SRPM URL: http://data-strzibny.rhcloud.com/obs/rubygem-clockwork-0.7.7-3.fc22.src.rpm -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1121082] Review Request: rubygem-clockwork - A scheduler process to replace cron
https://bugzilla.redhat.com/show_bug.cgi?id=1121082
František Dvořák changed:
What|Removed |Added
Flags|fedora-review? |fedora-review+
--- Comment #10 from František Dvořák ---
Package Review
==
Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
Issues:
===
- Sources used to build the package match the upstream source, as provided in
the spec URL.
Note: Upstream MD5sum check error, diff is in /home/valtri/fedora-
scm/REVIEWS/rubygem-clockwork/1121082-rubygem-clockwork/diff.txt
See: http://fedoraproject.org/wiki/Packaging/SourceURL
Updated manpage.
- You should add '-p' parameter to install commands to preserve timestamps.
= 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.
Note: Checking patched sources after %prep for licenses. Licenses found:
"Unknown or generated". 11 files have unknown license.
[x]: License file installed when any subpackage combination is installed.
[x]: Package must own all directories that it creates.
Note: Directories without known owners: /usr/share/gems,
/usr/share/gems/doc
[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
[x]: 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]: 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.
Note: Documentation size is 0 bytes in 0 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).
= SHOULD items =
Generic:
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Avoid bundling fonts in non-fonts packages.
Note: Package contains font files
They're in the generated docs.
[x]: If the source package does not include license text(s) as a separate file
from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[x]: Fully versioned dependency in subpackages if applicable.
[x]: Package functio
[Bug 1121082] Review Request: rubygem-clockwork - A scheduler process to replace cron
https://bugzilla.redhat.com/show_bug.cgi?id=1121082 Josef Stribny changed: What|Removed |Added Flags||fedora-cvs? --- Comment #11 from Josef Stribny --- Thank you for the review. New Package SCM Request === Package Name: rubygem-clockwork Short Description: A scheduler process to replace cron Upstream URL: http://github.com/tomykaira/clockwork Owners: jstribny Branches: f21 InitialCC: -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1121082] Review Request: rubygem-clockwork - A scheduler process to replace cron
https://bugzilla.redhat.com/show_bug.cgi?id=1121082 Jon Ciesla changed: What|Removed |Added Flags|fedora-cvs? |fedora-cvs+ -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1121082] Review Request: rubygem-clockwork - A scheduler process to replace cron
https://bugzilla.redhat.com/show_bug.cgi?id=1121082 --- Comment #12 from Jon Ciesla --- Git done (by process-git-requests). -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1121082] Review Request: rubygem-clockwork - A scheduler process to replace cron
https://bugzilla.redhat.com/show_bug.cgi?id=1121082 Josef Stribny changed: What|Removed |Added Status|NEW |CLOSED Fixed In Version||rubygem-clockwork-0.7.7-3.f ||c22 Resolution|--- |RAWHIDE Last Closed||2014-08-27 03:34:06 -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
