This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 682545 - Review Request: nutrika - Nutrient content calculator and intake analyzer
Review Request: nutrika - Nutrient content calculator and intake analyzer
Status: CLOSED NOTABUG
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
unspecified Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
:
Depends On:
Blocks: FE-DEADREVIEW
  Show dependency treegraph
 
Reported: 2011-03-06 10:01 EST by Péter Sulyok
Modified: 2012-07-02 11:45 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-07-02 11:45:29 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Péter Sulyok 2011-03-06 10:01:04 EST
Spec URL: http://peti.fedorapeople.org/nutrika.spec
SRPM URL: http://peti.fedorapeople.org/nutrika-0.2.1-1.fc14.src.rpm
Description: Shows detailed nutrition content (per 100g and DRI percentages) of various food items. Values of food items are also summed up for custom made meals,
and for a period according to a schedule.

This is my first package, and I need a sponsor.
Comment 1 Pavel Zhukov 2011-03-06 12:18:51 EST
Only few comments:

You're using RPM_BUILD_ROOT (variable style) and ?_smp_mflags (macro style)  Mixing the two styles, while valid, is bad from a QA and usability point of view, and should not be done in Fedora packages.
http://fedoraproject.org/wiki/Packaging/Guidelines#Using_.25.7Bbuildroot.7D_and_.25.7Boptflags.7D_vs_.24RPM_BUILD_ROOT_and_.24RPM_OPT_FLAGS

License is GPLv3+ 



-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers
Comment 2 Ralf Corsepius 2011-03-06 12:27:51 EST
(In reply to comment #1)
> Only few comments:
> 
> You're using RPM_BUILD_ROOT (variable style) and ?_smp_mflags (macro style) 
> Mixing the two styles, while valid, is bad from a QA and usability point of
> view, and should not be done in Fedora packages.
You are confused:

The only variables which must not be mixed within a spec file is 
$RPM_BUILD_ROOT and %buildroot.

There is nothing wrong with mixing %{?_smp_mflags} and $RPM_BUILD_ROOT.
Comment 3 Pavel Zhukov 2011-03-06 12:46:27 EST
Quote from Guidelines
"Mixing the two STYLES (not variables), while valid, is bad from a QA and usability point of view, and should not be done in Fedora packages."



-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers
Comment 4 Carlo Teubner 2011-03-06 15:16:32 EST
You need to use desktop-file-install: http://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files

There's no need to explicitly list sqlite and gtk2 as "Requires:". The version of sqlite that came with Fedora 14 is 3.6.23.1, and for gtk2 it's 2.22.0. Since you appear to target F14 only, an explicit requires is unnecessary, and indeed discouraged in the guidelines: http://fedoraproject.org/wiki/Packaging/Guidelines#Requires

BuildRoot is unnecessary: http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag

%clean is unnecessary: http://fedoraproject.org/wiki/Packaging/Guidelines#.25clean
Comment 5 Pavel Zhukov 2011-03-06 16:06:45 EST
> BuildRoot is unnecessary:
> http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag
It is necessary for EPEL for example
> %clean is unnecessary:
> http://fedoraproject.org/wiki/Packaging/Guidelines#.25clean
It's necessary for EPEL as well. 




-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers
Comment 6 Carlo Teubner 2011-03-06 16:33:32 EST
True. I had assumed this was only for Fedora 14 because of the "Release: 1.fc14". Perhaps better to use %{dist} to make it generic? Then again, this package is perhaps not that likely to end up in EPEL.
Comment 7 Péter Sulyok 2011-03-07 17:55:26 EST
I have
1) changed RPM_BUILD_ROOT to %{buildroot} for a consistent macro style
2) corrected License to GPLv3+
3) applied desktop-file-validate from desktop-file-install
4) removed unnecessary Requires tags
5) kept BuildRequires: gtk2-devel,sqlite-devel,gettext,desktop-file-utils, because it is necessary for mock build
6) removed BuildRoot tag, cleaning in %install, and the %clean section (I do not expect demand from EPEL5-)
Comment 8 Pavel Zhukov 2011-03-09 14:37:20 EST
I'll review it



-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers
Comment 9 Pavel Zhukov 2011-03-11 01:34:38 EST
Sorry, You need a sponsor, I cannot become sponsor.
Comment 10 Raghu Udiyar 2011-03-13 11:06:01 EDT
Hello Péter,

Please post the new SPEC/SRPM along with the rpmlint output and the koji build.

Or is it the same link above? I see the changes, but the changelog and release haven't been updated.
Comment 11 Péter Sulyok 2011-03-14 14:37:38 EDT
(In reply to comment #10)
> Please post the new SPEC/SRPM along with the rpmlint output and the koji build.
> Or is it the same link above? I see the changes, but the changelog and release
> haven't been updated.

I did not change the version/release number of the SPEC/SRPM just updated the previous files at http://peti.fedorapeople.org/

The rpmlint output is the following:
[peti@sutty SPECS]$ rpmlint nutrika.spec ../SRPMS/nutrika-0.2.1-1.fc14.src.rpm ../*/*/*.rpm
nutrika.x86_64: W: no-manual-page-for-binary nutrika
3 packages and 1 specfiles checked; 0 errors, 1 warnings.

This is the link to koji:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2911747
Comment 12 Raghu Udiyar 2011-03-14 20:21:29 EDT
* The release has to be updated every time the spec file is changed, with the appropriate changelog entries, see :

http://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs

> nutrika.x86_64: W: no-manual-page-for-binary nutrika

* Please work with upstream to add a manpage  :

http://fedoraproject.org/wiki/Packaging:Guidelines#Man_pages
Comment 13 Péter Sulyok 2011-03-15 12:12:34 EDT
I increased release number, added man page and changelog line for the fixes since rel 1. This is the rpmlint output now:
[peti@sutty rpm]$ rpmlint SPECS/nutrika.spec RPMS/x86_64/nutrika-* SRPMS/nutrika-0.2.1-2.fc14.src.rpm 
3 packages and 1 specfiles checked; 0 errors, 0 warnings.

The latest koji link is:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2913942

The spec file at http://peti.fedorapeople.org/ is also updated, and the latest src.rpm uploaded.
Comment 14 Jason Tibbitts 2012-06-29 21:21:17 EDT
I'm just looking over some of the older NEEDSPONSOR tickets and came across this one.  Not sure why it hasn't gotten much attention; the package is clean and builds fine.  After a quick glance, I have only a couple of comments:

There is no need to manually compress the manual page; rpm will take care of that for you.

The tarball in the srpm is not the same as what I get when I download Source0.  rpmlint complains thusly:

nutrika.src: W: file-size-mismatch nutrika-0.2.1.tar.bz2 = 5109185, http://sourceforge.net/projects/nutrika/files/nutrika-0.2.1.tar.bz2 = 5107037

and this is confirmed when I actually do the download.  Diffing them reveals several differences.  You should update from the current tarball and make sure to yell loudly at upstream for changing the download after they originally created it.

Anyway, if you are still interested in submitting this package after all this time, just let me know and I'll try to help you finish this up.
Comment 15 Péter Sulyok 2012-07-01 09:13:22 EDT
(In reply to comment #14)
I'm upstream. Apparently, I forgot to increase the version number when I did the changes for the Fedora package. 
Nutrika is an abandoned project now, unless someone wants to continue. It didn't meet public interest either. (The win package had about 7 dowloads/month.)
Thank you for the replies.
This ticket should be closed. I'm just not sure what option should I choose for the status.
Comment 16 Jason Tibbitts 2012-07-02 11:45:29 EDT
I'll close it out for you.

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