[Bug 1426965] Review Request: golang-github-nicksnyder-go-i18n - Translate Go programs into multiple languages

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1426965



--- Comment #4 from Fabio Valentini <decathorpe@xxxxxxxxx> ---
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
_______________________________________________
package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]