(This is not official review.)

Hi, I am a short comment on your spec file.

I don't know why you wrote the Source0 like this.

> # This URL: https://github.com/01org/dleyna-server/archive/v%{version}.tar.gz
> # will create a dleyna-server-%{version}.tar.gz file
> Source0:        %{name}-%{version}.tar.gz

If you have your code archive on your github, shouldn't you write url like [1]?

[1] https://fedoraproject.org/wiki/Packaging:SourceURL#Github

(In reply to Kenjiro Nakayama from comment #1)
> (This is not official review.)
>
> I don't know why you wrote the Source0 like this.
> 
> > # This URL: https://github.com/01org/dleyna-server/archive/v%{version}.tar.gz
> > # will create a dleyna-server-%{version}.tar.gz file
> > Source0:        %{name}-%{version}.tar.gz
> 
> If you have your code archive on your github, shouldn't you write url like
> [1]?
> 
> [1] https://fedoraproject.org/wiki/Packaging:SourceURL#Github

The format suggested in the guidelines do not really work. https://github.com/$OWNER/$PROJECT/archive/%{commit}/%{name}-%{version}-%{shortcommit}.tar.gz does not refer to a valid location if I replace $OWNER with 01org and $PROJECT with dleyna-server.

But mentioning the commit hash corresponding to the release sounds like a good idea, so I have done that.

Spec: http://rishi.fedorapeople.org/dleyna-server.spec
SRPM: http://rishi.fedorapeople.org/dleyna-server-0.4.0-2.fc20.src.rpm

Fedora review dleyna-server-0.4.0-2.fc20.src.rpm 2014-02-14

$ rpmlint dleyna-server \
          dleyna-server-debuginfo \
          dleyna-server-0.4.0-2.fc20.src.rpm
dleyna-server.src: W: invalid-url Source0: dleyna-server-0.4.0.tar.gz
3 packages and 0 specfiles checked; 0 errors, 1 warnings.

+ OK
! needs attention

+ The package is named according to Fedora packaging guidelines
+ The spec file name matches the base package name.
+ The package meets the Packaging Guidelines
+ The package is licensed with a Fedora approved license and meets the
  Licensing Guidelines.
+ The license field in the spec file matches the actual license
+ The package contains the license file (COPYING)
+ Spec file is written in American English
+ Spec file is legible
+ Upstream sources match the sources in the srpm
+ The package builds in koji
n/a ExcludeArch bugs filed
+ BuildRequires look sane
n/a The spec file handles locales properly
n/a ldconfig in %post and %postun
+ Package does not bundle copies of system libraries
n/a Package isn't relocatable
+ Package owns all the directories it creates
+ No duplicate files in %files
+ Permissions are properly set
+ Consistent use of macros
+ The package must contain code or permissible content
n/a Large documentation files should go in -doc subpackage
+ Files marked %doc should not affect package
n/a Header files should be in -devel
n/a Static libraries should be in -static
n/a Library files that end in .so must go in a -devel package
n/a -devel must require the fully versioned base
+ Packages should not contain libtool .la files
n/a Proper .desktop file handling
+ Doesn't own files or directories already owned by other packages
+ Filenames are valid UTF-8


Looks good to me, just a small issue with the Source URL. As yaneti pointed out on IRC, the way to make the github URLs work (as recommended in the packaging guidelines) would be to use the git hash that the tag points to. Right now you are using the hash of the tag object; should be the hash that the commit points to.

$ git show-ref --dereference v0.4.0
0e9d8bd48f80daea4f352a33ba1aa35af180dd46 refs/tags/v0.4.0
3fcae066b44195c187b5611acfd511b9a87850d0 refs/tags/v0.4.0^{}

^^ e.g. the 2nd hash here.

This is just a minor issue and won't block the package from getting imported, but would be great if you could fix it so that 'spectool -g' starts working.

APPROVED

(In reply to Kalev Lember from comment #3)

Thanks for the review!

> Looks good to me, just a small issue with the Source URL. As yaneti pointed
> out on IRC, the way to make the github URLs work (as recommended in the
> packaging guidelines) would be to use the git hash that the tag points to.
> Right now you are using the hash of the tag object; should be the hash that
> the commit points to.
> 
> $ git show-ref --dereference v0.4.0
> 0e9d8bd48f80daea4f352a33ba1aa35af180dd46 refs/tags/v0.4.0
> 3fcae066b44195c187b5611acfd511b9a87850d0 refs/tags/v0.4.0^{}
> 
> ^^ e.g. the 2nd hash here.

I see. The guidelines need to be updated in that case.
 
> This is just a minor issue and won't block the package from getting
> imported, but would be great if you could fix it so that 'spectool -g'
> starts working.

Fixed.

Spec: http://rishi.fedorapeople.org/dleyna-server.spec
SRPM: http://rishi.fedorapeople.org/dleyna-server-0.4.0-3.fc20.src.rpm

New Package SCM Request
=======================
Package Name: dleyna-server
Short Description: Service for interacting with Digital Media Servers
Owners: rishi
Branches: 
InitialCC:

1. autoreconf -f -i

-->

autoreconf -fiv

2. find $RPM_BUILD_ROOT -name '*.la' -delete

Please append -print so we can track what files have been deleted.

Git done (by process-git-requests).

-https://github.com/01org/%{version}/archive/%{commit}/%{name}-%{version}-%{shortcommit}.tar.gz
+https://github.com/01org/%{name}/archive/%{commit}/%{name}-%{version}-%{shortcommit}.tar.gz

Otherwise looks good to me! (And yes, I agree that the github URL guidelines could use a overhaul.)

(In reply to Christopher Meng from comment #6)
> 1. autoreconf -f -i
> 
> -->
> 
> autoreconf -fiv

Ok.
 
> 2. find $RPM_BUILD_ROOT -name '*.la' -delete
> 
> Please append -print so we can track what files have been deleted.

Ok.

(In reply to Kalev Lember from comment #8)
> -https://github.com/01org/%{version}/archive/%{commit}/%{name}-%{version}-
> %{shortcommit}.tar.gz
> +https://github.com/01org/%{name}/archive/%{commit}/%{name}-%{version}-
> %{shortcommit}.tar.gz

Oops! How did I manage to do that. Sorry about that.

Spec: http://rishi.fedorapeople.org/dleyna-server.spec
SRPM: http://rishi.fedorapeople.org/dleyna-server-0.4.0-4.fc20.src.rpm

Built for Rawhide. Thanks everybody!