- This one can be removed: docker/README.md

> %global godocs          docs CHANGELOG.md README.md docker/README.md


- Although correct, I think this time these can be conveted to "BuildRequires: /usr/bin/easyjson" type of BR.

> BuildRequires:  golang-github-mailru-easyjson
> BuildRequires:  golang-google-grpc
> BuildRequires:  golang-google-protobuf


- If you plan to use `make` you need a BR on it.


- Doesn't setting GO* env vars before make work?

> make GOPROXY=off GO111MODULE=off GOPATH=%{gobuilddir}:%{gopath} common


- Is client required? Makefile's build section only builds server and proxy.

> %gobuild -o %{gobuilddir}/bin/client %{goipath}/client

Documentation refers to it as "A simple client exists to benchmark the server."


- Change from this:

> %{_bindir}/*

to: 

%{_bindir}/signaling
%{_bindir}/proxy
%{_bindir}/client


- You can remove these to create a binary only package:

> %gopkginstall
> (...)
> %gopkgfiles


- IIUC this package is meant to be used as a daemon by default? If so systemd definition and user creation is missing.

https://github.com/strukturag/nextcloud-spreed-signaling/blob/master/README.md#running-as-daemon


- Does it make sense to split the package into signaling and proxy subpackages? Or add the proxy subpackage?

(In reply to Mikel Olasagasti Uranga from comment #1)
> - This one can be removed: docker/README.md
> 
> > %global godocs          docs CHANGELOG.md README.md docker/README.md

Done.

> - Although correct, I think this time these can be conveted to
> "BuildRequires: /usr/bin/easyjson" type of BR.
> 
> > BuildRequires:  golang-github-mailru-easyjson
> > BuildRequires:  golang-google-grpc
> > BuildRequires:  golang-google-protobuf

Done.

> - If you plan to use `make` you need a BR on it.

Done.
 
> - Doesn't setting GO* env vars before make work?
> 
> > make GOPROXY=off GO111MODULE=off GOPATH=%{gobuilddir}:%{gopath} common

No; still getting build errors:

Executing(%build): /bin/sh -e /var/tmp/rpm-tmp.mz5mbd
+ umask 022
+ cd /builddir/build/BUILD
+ cd nextcloud-spreed-signaling-1.2.4
+ GOPROXY=off
+ GO111MODULE=off
+ GOPATH=/builddir/build/BUILD/nextcloud-spreed-signaling-1.2.4/_build:/usr/share/gocode
+ make common
protoc \
        --go_out=. --go_opt=paths=source_relative \
        grpc_backend.proto
protoc \
        --go_out=. --go_opt=paths=source_relative \
        grpc_internal.proto
protoc \
        --go_out=. --go_opt=paths=source_relative \
        grpc_mcu.proto
protoc \
        --go_out=. --go_opt=paths=source_relative \
        grpc_sessions.proto
protoc \
        --go-grpc_out=. --go-grpc_opt=paths=source_relative \
        grpc_backend.proto
protoc \
        --go-grpc_out=. --go-grpc_opt=paths=source_relative \
        grpc_internal.proto
protoc \
        --go-grpc_out=. --go-grpc_opt=paths=source_relative \
        grpc_mcu.proto
protoc \
        --go-grpc_out=. --go-grpc_opt=paths=source_relative \
        grpc_sessions.proto
rm -f easyjson-bootstrap*.go
easyjson -all api_async.go
Error parsing api_async.go: file '/builddir/build/BUILD/nextcloud-spreed-signaling-1.2.4/api_async.go' is not in GOPATH '/builddir/build/BUILD/nextcloud-spreed-signaling-1.2.4/_build:/usr/share/gocode'
make: *** [Makefile:102: api_async_easyjson.go] Error 1

The full log is at: https://paste.centos.org/view/08c1fffe

> - Is client required? Makefile's build section only builds server and proxy.
> 
> > %gobuild -o %{gobuilddir}/bin/client %{goipath}/client
> 
> Documentation refers to it as "A simple client exists to benchmark the
> server."

It is needed for the tests. I don't think it is needed in the actual package. I don't think proxy is required at all either. 
 
> 
> - Change from this:
> 
> > %{_bindir}/*
> 
> to: 
> 
> %{_bindir}/signaling
> %{_bindir}/proxy
> %{_bindir}/client

Done.

> - You can remove these to create a binary only package:
> 
> > %gopkginstall
> > (...)
> > %gopkgfiles

Done.

> - IIUC this package is meant to be used as a daemon by default? If so
> systemd definition and user creation is missing.
> 
> https://github.com/strukturag/nextcloud-spreed-signaling/blob/master/README.
> md#running-as-daemon

You are right. I haven't included the systemd service unit. I will do this as soon as I can make the package build.

> - Does it make sense to split the package into signaling and proxy
> subpackages? Or add the proxy subpackage?

I think it is worth splitting. You don't always need a proxy. I will do this as soon as I can make the package build.

OK, package built thanks to Mikel's help. Thanks a lot. I have implemented all your indications.

I have added a sysusers configuration file.

OK, so, what's next?

This is an automatic check from review-stats script.

This review request ticket hasn't been updated for some time. We're sorry
it is taking so long. If you're still interested in packaging this software
into Fedora repositories, please respond to this comment clearing the
NEEDINFO flag.

You may want to update the specfile and the src.rpm to the latest version
available and to propose a review swap on Fedora devel mailing list to increase
chances to have your package reviewed. If this is your first package and you
need a sponsor, you may want to post some informal reviews. Read more at
https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group.

Without any reply, this request will shortly be considered abandoned
and will be closed.
Thank you for your patience.

Yep. I'm interested.