[Bug 1426965] Review Request: golang-github-nicksnyder-go-i18n - Translate Go programs into multiple languages
https://bugzilla.redhat.com/show_bug.cgi?id=1426965
--- Comment #4 from Fabio Valentini <decathorpe(a)gmail.com> ---
Initial comments:
1) I think the peculiar definition of %gobuild (at line 18) is necessary for
the successful generation of debuginfo packages, so that should be OK. Maybe
add a comment why it's there?
2) Since the package provides a binary (goi18n), I think the main package name
could be just named "goi18n" (see [1]) with subpackages
"golang-github-nicksnyder-go-i18n-devel" and
"golang-github-nicksnyder-go-i18n-unit-test-devel" (just like right now). But
using the appropriate "Provides:" for the main package name (like you do now)
should be fine too.
3) Please leave the %commit and %shortcommit definitions in the spec (just
paste the commit hash of the used tag) - this is used by some of the automatic
tooling for version tracking, for example like this:
# version 1.7.0 == commit f757c9f9b69c16ff69d38dbf224be28a7b6537bb
%global commit f757c9f9b69c16ff69d38dbf224be28a7b6537bb
%global shortcommit %(c=%{commit}; echo ${c:0:7})
4) What is the purpose of the %if-else block at line 45? There are no bundled
sources present, so the BR on golang(gopkg.in/yaml.v2) should be there
unconditionally.
5) Since the main package BRs: golang(gopkg.in/yaml.v2) anyway, you can remove
it the -devel subpackage (line 61) along with the enclosing %if-endif block.
6) You can remove the empty %if-endif block at line 82-85.
7) Since there are no bundled sources (and both branches of the if-else are
identical anyway), you can remove the wrapping around the GOPATH definition in
line 106/109.
8) Why are you including a commented-out build / install / %files entry of the
codegen binary? Maybe you could include a comment as to why you don't build
this binary, but the disabled build alone is not really informative / helpful.
[1]: https://fedoraproject.org/wiki/PackagingDrafts/Go#Packaging_Binaries
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component