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 173766
Summary: | Review Request: taarich - tell the Hebrew (Jewish) date | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Dan Kenigsberg <danken> |
Component: | Package Review | Assignee: | Michael A. Peters <mpeters> |
Status: | CLOSED NEXTRELEASE | QA Contact: | David Lawrence <dkl> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | fedora-extras-list |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
URL: | ftp://ftp.math.technion.ac.il/calendar/gauss/ | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2005-11-21 19:21:09 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: | |||
Bug Depends On: | |||
Bug Blocks: | 163779 |
Description
Dan Kenigsberg
2005-11-20 20:28:21 UTC
From the spec file: %build make - Will it build with the RPM_OPT_FLAGS ? - should use sed to add the $RPM_OPT_FLAGS to the CFLAGS in Makefile install -d %{buildroot}/{%{_docdir}/%{name},%{_bindir},%{_mandir}/man1} - Should be install -d -m755 %{buildroot}%{_bindir} install -d -m755 %{buildroot}%{_mandir}/man1 - You don't need to install the docdir - the %doc macro will do that for you. - Splitting it up instead of one liner makes it more readable with the macros - in there. gzip luach.man taarich.man cp luach.man.gz %{buildroot}%{_mandir}/man1/luach.1.gz cp taarich.man.gz %{buildroot}%{_mandir}/man1/taarich.1.gz - don't gzip the man pages, rpm will do it for you - use install to put the man pages in their place install -m644 {luach.man,taarich.man} %{buildroot}%{_mandir}/man1/ - will install them, and rpm will compress them. %files %defattr(-,root,root) %{_bindir}/taarich %{_bindir}/luach %doc gauss.txt reading.txt COPYING %{_mandir}/man1/taarich.1* %{_mandir}/man1/luach.1* - %doc is usually right below %defattr - %defattr needs to be changed to %defattr(-,root,root,-) - man pages need to be changed %files %defattr(-,root,root,-) %doc gauss.txt reading.txt COPYING %{_bindir}/taarich %{_bindir}/luach %{_mandir}/man1/* - would be much better Thank you for your suggestions. I applied all of them (I hope so), except for the one regarding sed; I did not quite understand what you wanted. Is it enough to pass make CFLAGS="$RPM_OPT_FLAGS -s" ? An updated SRPM sits in http://ivrix.org.il/redhat/taarich-1.20051120-2.src.rpm and the spec file is again in http://ivrix.org.il/redhat/taarich.spec (In reply to comment #2) > Thank you for your suggestions. I applied all of them (I hope so), > except for the one regarding sed; I did not quite understand what you > wanted. Is it enough to pass make CFLAGS="$RPM_OPT_FLAGS -s" ? Yeah - that works too. > > An updated SRPM sits in http://ivrix.org.il/redhat/taarich-1.20051120-2.src.rpm > and the spec file is again in http://ivrix.org.il/redhat/taarich.spec Formal review coming What's that "-s" in CFLAGS for? If it's for stripping binaries, it should be removed as it'll result in a useless -debuginfo package. (in reply to comment 4) Yep, that's my bad. I mistakenly understood that Michael A. Peters wanted that the upstream compilation flags would be maintained. Will be fixed during next take. (In reply to comment #5) > (in reply to comment 4) > Yep, that's my bad. I mistakenly understood that Michael A. Peters > wanted that the upstream compilation flags would be maintained. > Will be fixed during next take. With that change - Approved Good: + (from reviewer MUST section) * rpmlint output clean: [mpeters@utility result]$ ls build.log taarich-1.20051120-2.fc4.i386.rpm mockconfig.log taarich-1.20051120-2.fc4.src.rpm root.log taarich-debuginfo-1.20051120-2.fc4.i386.rpm [mpeters@utility result]$ rpmlint *.rpm [mpeters@utility result]$ * Package named according to guidelines - no upstream tarball name, named according to primary binary * Spec file name matches binary package name * Package meets packaging guidelines * OSI Approved license (MIT) - matches source * License in %doc * Spec file written in American English - Hebrew portions obviously appropriate, and english exists * Spec file legible * Sources match upstream - I mirrored upstream ftp directory to verify md5sum of files -- lftp -e 'mirror -e' ftp://ftp.math.technion.ac.il/calendar/gauss/ * Package succesfully compiles on i386 FC4 * No un-necessary BuildRequires, builds in mock * No lang files (no need for %find_lang) * No shared libraries (no need for ldconfig) * Proper use of macros for paths * No duplicate files * Proper %files section - appropriate permissions/ownership * Proper %clean * Consistent use of macros * Package contains code * No need for separate doc package * No need for devel package * No .la files * No need for desktop file + (from reviewer SHOULD section) * Builds in mock * Hebrew description * Package works [mpeters@utility result]$ taarich 19 Heshvan 5766 [mpeters@utility result]$ -=- Needs: There should be a Hebrew summary line since there is a Hebrew description. This is not a blocker, I'm approving. But adding Summary(he): Something in Hebrew under Summary: Tells the Hebrew date, Torah readings, and generates calendars would be advised. |