Bug 2077088 - Review Request: rubygem-scanf - A Ruby implementation of the C function scanf(3)
Summary: Review Request: rubygem-scanf - A Ruby implementation of the C function scanf(3)
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Breno
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2022-04-20 16:58 UTC by Ewoud Kohl van Wijngaarden
Modified: 2022-04-22 19:06 UTC (History)
3 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2022-04-22 19:06:35 UTC
Type: ---
Embargoed:
brandfbb: fedora-review+


Attachments (Terms of Use)

Description Ewoud Kohl van Wijngaarden 2022-04-20 16:58:05 UTC
Spec URL: http://ekohl.nl/rubygem-scanf.spec
SRPM URL: http://ekohl.nl/rubygem-scanf-1.0.0-1.fc37.src.rpm
Description: Packaging of https://rubygems.org/gems/scanf which is needed for Puppet. Initial packaging was done using gem2rpm, then modified as needed. The gem does not contain any tests nor any documentation. To solve the former I did a trivial require in %check, but the latter does result in a lint warning.
Fedora Account System Username: ekohl

Comment 1 Breno 2022-04-20 18:48:00 UTC
I know it's not super of a big deal, but there are diffs between the spec file inside the SRPM and the one in the URL. See below.
Also, we need a LICENSE file.

Other than that will be fine.


--- spec/rubygem-scanf.spec     2022-04-20 12:54:53.000000000 -0400
+++ srpm/rubygem-scanf.spec     2022-04-20 12:52:12.000000000 -0400
@@ -1,3 +1,12 @@
+## START: Set by rpmautospec
+## (rpmautospec version 0.2.5)
+%define autorelease(e:s:pb:) %{?-p:0.}%{lua:
+    release_number = 1;
+    base_release_number = tonumber(rpm.expand("%{?-b*}%{!?-b:1}"));
+    print(release_number + base_release_number - 1);
+}%{?-e:.%{-e*}}%{?-s:.%{-s*}}%{?dist}
+## END: Set by rpmautospec
+
 %global gem_name scanf
 
 Name: rubygem-%{gem_name}
@@ -57,4 +66,5 @@
 
 
 %changelog
-%autochangelog
+* Wed Apr 20 2022 Ewoud Kohl van Wijngaarden <ewoud> 1.0.0-1
+- Initial package

Comment 2 Ewoud Kohl van Wijngaarden 2022-04-20 19:02:22 UTC
(In reply to Breno from comment #1)
> I know it's not super of a big deal, but there are diffs between the spec
> file inside the SRPM and the one in the URL. See below.

I believe that is the result of the %autorelease and %autochangelog macros. Per https://fedoraproject.org/wiki/Changes/rpmautospec I believe this is the recommended approach.

> Also, we need a LICENSE file.

Should I for now copy the LICENSE file from upstream and open an issue/PR to include it in a future release? Perhaps that's also a place to ask about the test files and README.

Comment 3 Jarek Prokop 2022-04-20 19:15:46 UTC
One note, when gems do not ship with a test suite, it is often required to add the tests manually from the project's git, to run them during a build.

See for example rubygem-cucumber-wire: https://src.fedoraproject.org/rpms/rubygem-cucumber-wire/blob/rawhide/f/rubygem-cucumber-wire.spec#_13

Comment 4 Breno 2022-04-20 19:58:39 UTC
Hey Ewoud,

Yes, you can add the license manually You will be adding it to the repo, once we have one.

As per the rpmautospec, that's news to me. Weird that fedora-review is not up-to-date accordingly.

And for the tests, if the upstream provides tests from their repo, i think it's worth using them when building the package, like Jarked proposed.

Comment 5 Ewoud Kohl van Wijngaarden 2022-04-21 08:37:11 UTC
(In reply to Breno from comment #4)
> Yes, you can add the license manually You will be adding it to the repo,
> once we have one.

Ok, will do.

> As per the rpmautospec, that's news to me. Weird that fedora-review is not
> up-to-date accordingly.

https://bugzilla.redhat.com/show_bug.cgi?id=1985462

> And for the tests, if the upstream provides tests from their repo, i think
> it's worth using them when building the package, like Jarked proposed.

I'll investigate.

Comment 6 Ewoud Kohl van Wijngaarden 2022-04-21 09:31:47 UTC
To include the other files I opened https://github.com/ruby/scanf/pull/11, but I don't think it's likely it'll be merged and released soon enough (given I already broke Puppet in Rawhide) so I'll proceed with copying files for now.

Comment 7 Ewoud Kohl van Wijngaarden 2022-04-21 12:05:38 UTC
Updated with LICENSE and tests:

Spec URL: http://ekohl.nl/rubygem-scanf-with-files.spec
SRPM URL: http://ekohl.nl/rubygem-scanf-1.0.0-2.fc37.src.rpm

I wasn't sure about README so I left that out for now. Since it was just 4 files I opted to include the test files in the SRPM rather than a separate tarball like Jarek's example. Let me know if you prefer something else.

Comment 8 Breno 2022-04-22 14:30:12 UTC
LGTM, good work brother.

Comment 9 Breno 2022-04-22 14:30:33 UTC
Oh, btw, feel free to add me as a co-maintainer if you feel like it.

Comment 10 Ewoud Kohl van Wijngaarden 2022-04-22 15:43:12 UTC
Ok, will do. I created https://pagure.io/releng/fedora-scm-requests/issue/43834 so I'll wait until that's processed.

Comment 11 Gwyn Ciesla 2022-04-22 16:42:09 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/rubygem-scanf

Comment 12 Ewoud Kohl van Wijngaarden 2022-04-22 19:06:35 UTC
I uploaded the package and created the first build.


Note You need to log in before you can comment on or make changes to this bug.