Bug 471575 - (libnautilus-vcards) Review Request: libnautilus-vcards - Nautilus vcard context menu extension
Review Request: libnautilus-vcards - Nautilus vcard context menu extension
Status: CLOSED NOTABUG
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium 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: 2008-11-14 07:15 EST by Alec Leamas
Modified: 2009-07-10 14:22 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-07-10 14:22:10 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 Alec Leamas 2008-11-14 07:15:47 EST
Spec URL: ftp://mumin.dnsalias.net/pub/libnautilus-vcards.spec
SRPM URL: ftp://mumin.dnsalias.net/pub//libnautilus-vcards-0.1-23:26M.x86_64.src.rpm
Description: A Nautilus Vcard extension adding context menu items for sending email, call, send SMS or take a Tomboy note for users represented by a vcard. The different applications are command line apps configured through gconf.

Some more info at http://sourceforge.net/projects/dt-contacts.  This is  one of
two subprojects within this project.
Comment 1 Alec Leamas 2008-11-15 16:53:29 EST
Updated spec file and rpm. New RPM url: ftp://mumin.dnsalias.net/pub//libnautilus-vcards-0.1-34.x86_64.src.rpm (spec file the same).

Also, I really need a sponsor... This is, more or less, my first attempt to bring a package into Fedora.
Comment 2 Alec Leamas 2008-11-18 16:29:53 EST
I'm trying to do my homework... Reviewed some other packages, read still more other reviews. Updated the package, including spec file. See the changelog for more info.

New urls: 

http://downloads.sourceforge.net/dt-contacts/libnautilus-vcards-0.2-48.fc9.src.rpm and 

http://downloads.sourceforge.net/dt-contacts/libnautilus-vcards.spec
Comment 3 Fabian Affolter 2008-11-19 05:49:18 EST
Just some quick comment on your spec file

- remove the redundant stuff
  - %define version 0.2 -> Version: 0.2, later you can use %{version}. This is done automatically.
  - remove pkg (this is the macro %{name} ), url, and the dir stuff. The spec file will be much more readable. For almost all your entires exists a macro:
   https://fedoraproject.org/wiki/PackageMaintainers/CreatingPackageHowTo#Macros

- Release: %{svnversion}%{?dist} . This means the release of the spec file not the version of the software.  In your case this is '1' at the moment.  More details about the naming https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Package_Release

- URL should point to the project's website
  http://sourceforge.net/projects/dt-contacts/

- BuildRoot: Samples at https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag

- For translations is 'BR: gettext' sufficient.  'BuildRequires: gettext-devel' is not needed
  https://fedoraproject.org/wiki/Packaging/Guidelines#Handling_Locale_Files

- Replace '%defattr(-, root, root, 0755)' with '%defattr(-, root, root, -)'

- add ABOUT-NLS to %doc

- Is 'mkdir -p $RPM_BUILD_ROOT' needed in the %install section?
Comment 5 Alec Leamas 2008-11-19 06:35:47 EST
Oops, to late... I've done  bug 470066 in a more formal manner, and put remarks on bug 470547 and bug 470155
Comment 6 Alec Leamas 2008-11-19 07:05:31 EST
(Scrolling up, finding your previous post...)

Thanks for you remarks. Special thanks for the link on macros!  Most of the remarks is just to fix. But:

- In my case, the spec file is part of the source tree, living in the same
  svn repo as the source. I have actually thought about this... which 
  doesn't mean I'm sure. But to me it seems reasonable to use the svn 
  version in this case, since spec and source is always in sync. Or?

- Auto(re)conf needs the gettext m4 macros in gettext-devel
Comment 7 Alec Leamas 2008-11-19 08:16:38 EST
Made a new version. The issues you mentioned fixed, besides:

- Trying to find a balance in when to define macros I have 
  removed %pkg and %pkgdatadir, but kept %schemadir, %plugindir and
  %download_url. If you insist, I'll remove them. But I feel they make
  things a better e. g., by making the scriptlets a bit more more concise.
  And personally, I avoid source lines > 72 chars; hence %download_url

- gettext-devel still there (see above).

- And still svn-based revision. I'm not stubborn, but a little interested
  what the arguments are in a situation when the spec file release is the
  same as the source release... The reasons to stick to svn # are obvious,
  but I don't have to complete picture.

New spec url: 

http://downloads.sourceforge.net/dt-contacts/libnautilus-vcards-48M.spec

(The filename is bad, a cludge to handle sf limitations)
Comment 8 Fabian Affolter 2008-11-19 09:50:38 EST
(In reply to comment #5)
> Oops, to late... I've done  bug 470066 in a more formal manner, and put remarks
> on bug 470547 and bug 470155

Just to be clear, I'm not a sponsor but I can help you with this package.  It was just a suggestion about the reviews without any further investigation on your work.

(In reply to comment #6)
> - In my case, the spec file is part of the source tree, living in the same
>   svn repo as the source. I have actually thought about this... which 
>   doesn't mean I'm sure. But to me it seems reasonable to use the svn 
>   version in this case, since spec and source is always in sync. Or?

More about the release tag, see below.

> - Auto(re)conf needs the gettext m4 macros in gettext-devel

I just quoted the guidelines.  So, you are right.

(In reply to comment #7)
> - Trying to find a balance in when to define macros I have 
>   removed %pkg and %pkgdatadir, but kept %schemadir, %plugindir and
>   %download_url. If you insist, I'll remove them. But I feel they make
>   things a better e. g., by making the scriptlets a bit more more concise.
>   And personally, I avoid source lines > 72 chars; hence %download_url

I can live with that.  Spec files are just easier to read for other packages when only 'standard' macros are used.  
 
> - And still svn-based revision. I'm not stubborn, but a little interested
>   what the arguments are in a situation when the spec file release is the
>   same as the source release... The reasons to stick to svn # are obvious,
>   but I don't have to complete picture.

I think that your release is a post release.  The naming guidelines have some examples.
https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Post-Release_packages

"Release: The initial value of the release should normally be "1%{?dist}". Then, increment the number every time you release a new package for the same version of software. If a new version of the software being packaged is released, the version number should be changed to reflect the new software version, and the release number should be reset to 1."

From my point of view the release tag should look like this

Release:        1.%{svnversion}svn%{?dist}
 
> http://downloads.sourceforge.net/dt-contacts/libnautilus-vcards-48M.spec

This way the name of the spec file didn't match the guidelines.  I'm not an expert on exceptions.

https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Spec_file_name
Comment 9 Alec Leamas 2008-11-19 11:46:06 EST
The name of the spec file is still libnautilus-vcards.spec. I just had to rename it to handle some mysterious sf upload policies. It's just for the review.

Your naming scheme looks great! It makes it possible to keep everybody happy. I'll go for that. Thanks!

Many thanks for your help. I'll try to find some time for yet another review, this time one from your list...
Comment 10 Alec Leamas 2008-11-19 12:45:39 EST
When I looked at the generated RPM:s, with names like libnautilus-vcards-0.2-1.48Msvn.fc9.x86_64.rpm, I hesitated. It just doesn't look "right". 
These kind of names normally indicates some kind of snapshot or automatic, nightly builds. 

Read the release guidelines over again. And removed the 'svn' part i. e.,
1.48svn -> 1.48. To me, it looks better, and seems to comply with the
guidelines. Thus the release would be 0.2-1.54, 0.2-1.73, 0.3-1.85...

Which leaves an open issue: under what circumstances should the 1. prefix
change? My feeling is that will be left constantly as '1.'. 

Looking at the kernel (uname -r) , I suspect they have something like %version-%svnversion...

Eventually, maybe some sponsor could have a final word on this? For the time being, I  stick to %version-1.%&svnversion

New URL:

http://downloads.sourceforge.net/dt-contacts/libnautilus-vcards-53.spec

(and, the "real" name still is libnautilus-vcards.spec, but sourceforge has their policies...)
Comment 11 Mamoru TASAKA 2009-05-25 13:21:08 EDT
Well, as I cannot find where to download the needed Source
I cannot try to rebuild this package. Then:

- Please update your spec/srpm with addressing my comment
  on bug 472696 comment 3.
- For GConf schemas scriptlets, please refer to:
  https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#GConf
  ! Note that there are %pre, %post and %preun scriptlet. Also
    the corresponding Requires(pre) or so are needed.
- We usually does _not_ regard GConf schemas files under
  %_sysconfdir/gconf/schemas as configuration files.
  ( rpmlint will complain if you remove %config(noreplace) from
    GConf schemas file, however please just ignore the warning)
Comment 12 Mamoru TASAKA 2009-06-07 12:45:31 EDT
ping?
Comment 13 Mamoru TASAKA 2009-06-14 12:42:55 EDT
ping again?
Comment 14 Mamoru TASAKA 2009-06-28 15:12:45 EDT
I will close this bug if no response is received from the reporter
within ONE WEEK.
Comment 15 Mamoru TASAKA 2009-07-10 14:22:10 EDT
Closing.

If someone wants to import this package into Fedora, please
file a new review request and mark this bug as a duplicate
of the new one.

Thank you!

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