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. |