Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1049557
Summary: | Review Request: jpeginfo - Utility to obtain information from JPEG files | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Denis Fateyev <denis> |
Component: | Package Review | Assignee: | Igor Gnatenko <ignatenko> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | ignatenko, mattdm, package-review |
Target Milestone: | --- | Flags: | ignatenko:
fedora-review+
gwync: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | jpeginfo-1.6.1-1.el5 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2014-01-25 14:11:25 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: |
Description
Denis Fateyev
2014-01-07 18:15:20 UTC
Taken ;) Fast look at spec: BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root %if 0%{?el5} %clean rm -rf %{buildroot} %endif and the same.... If you don't want to package it for EL5 - drop it. %defattr(-,root,root,-) Not needed since F18 or older. install -Dpm 0755 jpeginfo %{buildroot}/%{_bindir}/jpeginfo install -Dpm 0644 jpeginfo.1 %{buildroot}/%{_mandir}/man1/jpeginfo.1 Why you using this? why not %make_install, which identical to make install DESTDIR=%{buildroot} ? - Fix incorrect FSF address in files COPIYNG, COPYRIGHT - Add Public Domain License to Licese-tag - Please remove getopt sources in prep section (for innocence) Igor, thanks for reviewing. >If you don't want to package it for EL5 - drop it. That's made intentionally since among other systems I'm planning to provide it for EPEL5. All el5-related stuff wrapped into conditionals. > %defattr(-,root,root,-) > Not needed since F18 or older. Same here - I left that also for Fedora since it doesn't break things. > Why you using this? why not %make_install, which identical to > make install DESTDIR=%{buildroot} ? Unfortunately, "make install" (and aliases) doen't work properly with the package due some vagueness in Makefile. I preferred to pick the needed things up to place them into the right location manually instead of patching Makefile. > Please remove getopt sources in prep section (for innocence) Didn't get that, sorry. Could you provide more details? > Fix incorrect FSF address in files COPIYNG, COPYRIGHT > Add Public Domain License to Licese-tag It's licensed under GPLv2 or later, cannot be considered as Public Domain ;-) As for the "incorrect FSF postal address", I really doubt it's an issue here. Funny, I was just thinking it'd be handy to have this packaged up. I'll let Igor finish the official review but throw in a few comments. It's not a big deal but I'd like to see more descriptive Summary -- something that mentions the error-checking capability, since we have a number of other utilities which do the listing and none that do the checking. I'd also suggest sneaking '.jpg' into the summary so that comes up in people's searches. Summary: Error-check and generate informative listings from JPEG files %description Jpeginfo prints information and tests integrity of JPEG/JFIF files. It can generate informative listings of .jpg files, and can also be used to test them for errors (and optionally delete broken files). or so. The license should be License: GPLv2+ since it includes the "or later" clause. I think we're missing some buildrequires, aren't we? BuildRequires: libjpeg-devel I don't think that there's any reason to remove the getopt files, but it also doesn't hurt. The idea is just to make sure that some glitch in the build process doesn't accidentally use those instead of the proper system version. There is an md5 copylib, which has an automatic extension but which should be noted -- see https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries -- which says to do: Provides: bundled(md5-plumb) (In this case, since it's the Colin Plumb code.) (In reply to Matthew Miller from comment #5) > Funny, I was just thinking it'd be handy to have this packaged up. I'll let > Igor finish the official review but throw in a few comments. > > It's not a big deal but I'd like to see more descriptive Summary -- > something that mentions the error-checking capability, since we have a > number of other utilities which do the listing and none that do the > checking. I'd also suggest sneaking '.jpg' into the summary so that comes up > in people's searches. Matthew, thanks for these pretty informative descriptions! I have just added them into the spec. I've also ajusted License tag, removed getopt files, and added Provides as requested. As for BuildRequires, "libjpeg-devel" is already presented there, seems there are no other specific dependencies. Please check the spec file if I'm wrong. Updated spec: http://www.fateyev.com/RPMS/Fedora19/testing/jpeginfo.spec Source RPM: http://www.fateyev.com/RPMS/Fedora19/testing/SRPMS/jpeginfo-1.6.1-1.fc19.denf.src.rpm I must have just overlooked the libjpeg-devel. Apologies! It all looks good to me. APPROVED. Sorry for the delay :( New Package SCM Request ======================= Package Name: jpeginfo Short Description: Error-check and generate informative listings from JPEG files Owners: dfateyev Branches: f19 f20 el5 el6 InitialCC: Git done (by process-git-requests). jpeginfo-1.6.1-1.el5 has been submitted as an update for Fedora EPEL 5. https://admin.fedoraproject.org/updates/jpeginfo-1.6.1-1.el5 jpeginfo-1.6.1-1.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/jpeginfo-1.6.1-1.el6 jpeginfo-1.6.1-1.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/jpeginfo-1.6.1-1.fc19 jpeginfo-1.6.1-1.fc20 has been submitted as an update for Fedora 20. https://admin.fedoraproject.org/updates/jpeginfo-1.6.1-1.fc20 jpeginfo-1.6.1-1.fc20 has been pushed to the Fedora 20 stable repository. jpeginfo-1.6.1-1.fc19 has been pushed to the Fedora 19 stable repository. jpeginfo-1.6.1-1.el6 has been pushed to the Fedora EPEL 6 stable repository. jpeginfo-1.6.1-1.el5 has been pushed to the Fedora EPEL 5 stable repository. Package Change Request ====================== Package Name: jpeginfo New Branches: epel7 Owners: dfateyev Git done (by process-git-requests). |