[Bug 1066176] Review Request: min-metadata-service - Client for EC2/OpenStack metadata
https://bugzilla.redhat.com/show_bug.cgi?id=1066176 Petr Pisar changed: What|Removed |Added Status|NEW |CLOSED CC||ppi...@redhat.com Fixed In Version||min-metadata-service-2014.3 ||-1.fc21 Resolution|--- |CURRENTRELEASE Last Closed||2020-04-23 11:42:16 -- 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 To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org
[Bug 1066176] Review Request: min-metadata-service - Client for EC2/OpenStack metadata
https://bugzilla.redhat.com/show_bug.cgi?id=1066176 Bug 1066176 depends on bug 1066359, which changed state. Bug 1066359 Summary: Review Request: libgsystem - GIO-based library with Unix/Linux specific API https://bugzilla.redhat.com/show_bug.cgi?id=1066359 What|Removed |Added Status|MODIFIED|CLOSED Resolution|--- |ERRATA -- 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 1066176] Review Request: min-metadata-service - Client for EC2/OpenStack metadata
https://bugzilla.redhat.com/show_bug.cgi?id=1066176 Colin Walters walt...@redhat.com changed: What|Removed |Added Flags||fedora-cvs? --- Comment #12 from Colin Walters walt...@redhat.com --- New Package SCM Request === Package Name: min-metadata-service Short Description: Minimal client implementation of EC2/OpenStack metadata API Owners: walters mattdm Branches: rawhide InitialCC: walt...@verbum.org -- 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 1066176] Review Request: min-metadata-service - Client for EC2/OpenStack metadata
https://bugzilla.redhat.com/show_bug.cgi?id=1066176 --- Comment #13 from Jon Ciesla limburg...@gmail.com --- InitialCC needs a FAs account, not email, and don't request rawhide, devel is automatic. -- 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 1066176] Review Request: min-metadata-service - Client for EC2/OpenStack metadata
https://bugzilla.redhat.com/show_bug.cgi?id=1066176 Jon Ciesla limburg...@gmail.com changed: What|Removed |Added Flags|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 1066176] Review Request: min-metadata-service - Client for EC2/OpenStack metadata
https://bugzilla.redhat.com/show_bug.cgi?id=1066176 Colin Walters walt...@redhat.com changed: What|Removed |Added Flags||fedora-cvs? --- Comment #14 from Colin Walters walt...@redhat.com --- New Package SCM Request === Package Name: min-metadata-service Short Description: Minimal client implementation of EC2/OpenStack metadata API Owners: walters mattdm Branches: 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 1066176] Review Request: min-metadata-service - Client for EC2/OpenStack metadata
https://bugzilla.redhat.com/show_bug.cgi?id=1066176 --- Comment #15 from Jon Ciesla limburg...@gmail.com --- 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 1066176] Review Request: min-metadata-service - Client for EC2/OpenStack metadata
https://bugzilla.redhat.com/show_bug.cgi?id=1066176 Jon Ciesla limburg...@gmail.com 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 1066176] Review Request: min-metadata-service - Client for EC2/OpenStack metadata
https://bugzilla.redhat.com/show_bug.cgi?id=1066176
--- Comment #10 from Matthew Miller mat...@redhat.com ---
Package Review
==
Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed
= MUST items =
C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.
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:
LGPL, Unknown or generated. 2 files have unknown license. Detailed
output of licensecheck in /home/mattdm/tmp/1066176-min-metadata-
service/licensecheck.txt
[x]: %build honors applicable compiler flags or justifies otherwise.
[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.
[x]: Package contains systemd file(s) if in need.
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[-]: Large documentation must go in a -doc subpackage. Large could be size
(~1MB) or number of files.
Note: Documentation size is 30720 bytes in 1 files.
[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 must own all directories that it creates.
[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]: Packages must not store files under /srv, /opt or /usr/local
= SHOULD items =
Generic:
[-]: 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]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: SourceX tarball generation or download is documented.
Note: Package contains tarball without URL, check comments
[ ]: Description and summary sections in the package spec file contains
translations for supported Non-English languages, if available.
[ ]: 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]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[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).
[Bug 1066176] Review Request: min-metadata-service - Client for EC2/OpenStack metadata
https://bugzilla.redhat.com/show_bug.cgi?id=1066176 Matthew Miller mat...@redhat.com changed: What|Removed |Added Flags|fedora-review? |fedora-review+ --- Comment #11 from Matthew Miller mat...@redhat.com --- Review passed. But fix the date in the changelog (should be Sun Mar 02 2014 or Sat Mar 01, whichever you meant) or else rpmlint will complain about it forever after. :) -- 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 1066176] Review Request: min-metadata-service - Client for EC2/OpenStack metadata
https://bugzilla.redhat.com/show_bug.cgi?id=1066176 --- Comment #9 from Colin Walters walt...@redhat.com --- SPEC: http://fedorapeople.org/~walters/min-metadata-service.spec SRPM: http://fedorapeople.org/~walters/min-metadata-service-2014.3-1.el7.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 1066176] Review Request: min-metadata-service - Client for EC2/OpenStack metadata
https://bugzilla.redhat.com/show_bug.cgi?id=1066176 --- Comment #6 from Matthew Miller mat...@redhat.com --- I hate to be picky, but the command in the spec comment should have the tag or commit that matches the version in the tarball. The goal is that someone should be able to create the exact same thing independently for verification, not just as a convenience for updating. And I also hate to be difficult about the changelog, but that's a must item in the packaging guidelines. Can you make the forked package for inclusion in Fedora at this point? That's what's supposed to be being reviewed, in any case. Those points aside, the actual thing looks good, so I'll pass the review as soon as it's available. -- 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 1066176] Review Request: min-metadata-service - Client for EC2/OpenStack metadata
https://bugzilla.redhat.com/show_bug.cgi?id=1066176 --- Comment #7 from Colin Walters walt...@redhat.com --- (In reply to Matthew Miller from comment #6) I hate to be picky, but the command in the spec comment should have the tag or commit that matches the version in the tarball. The goal is that someone should be able to create the exact same thing independently for verification, not just as a convenience for updating. Being picky is good! Particularly so I can show you how I do things, and you'll be better able to understand the model when I get around to some proposals for changes on the Fedora/RPM source side. So the source tarball name with Makefile.dist-packaging is always named after a variant of git describe. In this case, the tarball is being built from a tag named v2014.1, so the version is just 2014.1. If I make a tarball from a later commit, it looks like this: ostree-2014.1.36.gb762c2f.tar.xz That means we're 36 commits after the tag 2014.1, and the commit is b762c2f. And I also hate to be difficult about the changelog, but that's a must item in the packaging guidelines. For now... =) Can you make the forked package for inclusion in Fedora at this point? That's what's supposed to be being reviewed, in any case. Sure. -- 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 1066176] Review Request: min-metadata-service - Client for EC2/OpenStack metadata
https://bugzilla.redhat.com/show_bug.cgi?id=1066176 --- Comment #8 from Matthew Miller mat...@redhat.com --- (In reply to Colin Walters from comment #7) So the source tarball name with Makefile.dist-packaging is always named after a variant of git describe. In this case, the tarball is being built from a tag named v2014.1, so the version is just 2014.1. That seems reasonable, but the point of the comment is so the person looking doesn't have to figure out of you are reasonable or not, and in which of several perfectly valid ways *you* are reasonable. :) So it should go into the specfile comment. And I also hate to be difficult about the changelog, but that's a must item in the packaging guidelines. For now... =) A lot of *all* of this is about a separation between developer and packager roles. You're acting as both, and I think have a bias towards the developer side. That's not inherently bad, but tilting the way we do things in that direction is... well, too big of a conversation for this bug. :) -- 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 1066176] Review Request: min-metadata-service - Client for EC2/OpenStack metadata
https://bugzilla.redhat.com/show_bug.cgi?id=1066176 --- Comment #5 from Colin Walters walt...@redhat.com --- (In reply to Matthew Miller from comment #2) I ran into a basic showstopper because this depends on pkgconfig(libgsystem), and it looks like libgsystem is Not Yet A Thing. It is now A Thing =) http://koji.fedoraproject.org/koji/buildinfo?buildID=499398 There is no %changelog. We need one. I don't have %changelog in the spec files I maintain inside the modules, since git operates as a perfectly fine changelog. When the spec file gets forked for inclusion in Fedora, I'll add one per the current requirements. Fixed everything else, thanks! https://github.com/cgwalters/min-metadata-service/commit/410d7024fdcdcdbb5d31c709b3dd389b0ea68e43 https://github.com/cgwalters/min-metadata-service/commit/dd344bd58a27e6875badc92fea264d245594ae2b -- 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 1066176] Review Request: min-metadata-service - Client for EC2/OpenStack metadata
https://bugzilla.redhat.com/show_bug.cgi?id=1066176 --- Comment #4 from Colin Walters walt...@redhat.com --- (In reply to Sandro Mathys from comment #3) Using this source URL (and the tarball that comes from it) is clearly preferred over using your self-created tarball unless there's good reasons against it (which you also should add to the comment preceding the source non-URL). I have generic infrastructure (in the Makefile.dist-packaging) for generating tarballs via git archive that I use for most of my projects. The approach above only works for Github - not all of my projects are there. -- 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 1066176] Review Request: min-metadata-service - Client for EC2/OpenStack metadata
https://bugzilla.redhat.com/show_bug.cgi?id=1066176 Matthew Miller mat...@redhat.com changed: What|Removed |Added CC||mat...@redhat.com Assignee|nob...@fedoraproject.org|mat...@redhat.com Flags||fedora-review? --- Comment #1 from Matthew Miller mat...@redhat.com --- FTR, it makes reviewing easier if you actually provide the spec and source RPM, because fedora-review is expecting them. But I'll take a look anyway. :) -- 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 1066176] Review Request: min-metadata-service - Client for EC2/OpenStack metadata
https://bugzilla.redhat.com/show_bug.cgi?id=1066176 Matthew Miller mat...@redhat.com changed: What|Removed |Added Depends On||1066359 --- Comment #2 from Matthew Miller mat...@redhat.com --- I ran into a basic showstopper because this depends on pkgconfig(libgsystem), and it looks like libgsystem is Not Yet A Thing. Other things: Since you don't have an upstream source URL, you should precede that line with a comment explaining how to generate the tar.xz. See https://fedoraproject.org/wiki/Packaging:SourceURL?rd=Packaging/SourceURL The %clean section and rm -rf $RPM_BUILD_ROOT aren't needed unless this is going into EPEL 5. For Fedora and newer epel, these are unneeded. Ditto %defattr. There is no %changelog. We need one. Need require systemd, because of the .service file. (More specifically, for the ownership of the directory that file goes in.) Very minor: I'd like to see the Summary be a little more descriptive. And, currently, doesn't seem to mention OpenStack in either the summary or description. Also: your COPYING File has the wrong address for the FSF. Referenced Bugs: https://bugzilla.redhat.com/show_bug.cgi?id=1066359 [Bug 1066359] Review Request: libgsystem - GIO-based library with Unix/Linux specific API -- 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 1066176] Review Request: min-metadata-service - Client for EC2/OpenStack metadata
https://bugzilla.redhat.com/show_bug.cgi?id=1066176 Sandro Mathys s...@sandro-mathys.ch changed: What|Removed |Added CC||s...@sandro-mathys.ch --- Comment #3 from Sandro Mathys s...@sandro-mathys.ch --- (In reply to Matthew Miller from comment #2) Since you don't have an upstream source URL, you should precede that line with a comment explaining how to generate the tar.xz. See https://fedoraproject.org/wiki/Packaging:SourceURL?rd=Packaging/SourceURL Actually, as can be read on the referred page, it's easily possible to provide a valid source URL as Github offers automated archiving of both tags and revisions: https://fedoraproject.org/wiki/Packaging:SourceURL#Github Using this source URL (and the tarball that comes from it) is clearly preferred over using your self-created tarball unless there's good reasons against it (which you also should add to the comment preceding the source non-URL). -- 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
