Bug 1359802 - Review Request: golang-github-gosexy-gettext - Gettext support for the Go language
Summary: Review Request: golang-github-gosexy-gettext - Gettext support for the Go lan...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Neal Gompa
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-07-25 13:17 UTC by Zygmunt Krynicki
Modified: 2016-08-27 10:45 UTC (History)
3 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2016-08-18 00:49:56 UTC
Type: ---
Embargoed:
ngompa13: fedora-review+


Attachments (Terms of Use)

Description Zygmunt Krynicki 2016-07-25 13:17:49 UTC
Spec URL: https://fedorapeople.org/~zyga/golang-github-gosexy-gettext.spec
SRPM URL: https://fedorapeople.org/~zyga/golang-github-gosexy-gettext-0-0.1.git98b7b91.fc24.src.rpm
Description:
Go bindings for GNU gettext, an internationalization and localization library
for writing multilingual systems.

Fedora Account System Username: zyga

Comment 1 Neal Gompa 2016-07-25 17:12:26 UTC
Taking this review.

Comment 2 Neal Gompa 2016-07-25 17:22:42 UTC
A cursory check of this package sources indicate that it depends on gettext (maybe gettext-devel?) to and "golang(github.com/jessevdk/go-flags)" to build. Neither appear present in your spec.

Are you sure this is set up right?

Comment 3 Zygmunt Krynicki 2016-07-25 19:09:51 UTC
(In reply to Neal Gompa from comment #2)
> A cursory check of this package sources indicate that it depends on gettext
> (maybe gettext-devel?) to and "golang(github.com/jessevdk/go-flags)" to
> build. Neither appear present in your spec.
> 
> Are you sure this is set up right?

This is actually (confusingly) correct.

Checking the code we simply link to libc, not to anything specific in gettext-devel.

Ah, I missed the go-flags comment. How did you find this? AFAIK nothing imports go-flags in the source package (I just gripped for it)


^^ - just comments I wrote as a reply to the bugzilla email (sorry, not used to the process yet).

Best regards
ZK

Comment 4 Neal Gompa 2016-07-25 19:25:10 UTC
(In reply to Zygmunt Krynicki from comment #3)
> (In reply to Neal Gompa from comment #2)
> > A cursory check of this package sources indicate that it depends on gettext
> > (maybe gettext-devel?) to and "golang(github.com/jessevdk/go-flags)" to
> > build. Neither appear present in your spec.
> > 
> > Are you sure this is set up right?
> 
> This is actually (confusingly) correct.
> 
> Checking the code we simply link to libc, not to anything specific in
> gettext-devel.
>

Weird, the readme seems to imply that it needs to link to gettext-devel...

> Ah, I missed the go-flags comment. How did you find this? AFAIK nothing
> imports go-flags in the source package (I just gripped for it)
> 
> 

I saw the go-flags import here: https://github.com/gosexy/gettext/blob/master/go-xgettext/main.go#L42

Comment 5 Zygmunt Krynicki 2016-07-25 19:28:01 UTC
(In reply to Neal Gompa from comment #4)
> (In reply to Zygmunt Krynicki from comment #3)
> > (In reply to Neal Gompa from comment #2)
> > > A cursory check of this package sources indicate that it depends on gettext
> > > (maybe gettext-devel?) to and "golang(github.com/jessevdk/go-flags)" to
> > > build. Neither appear present in your spec.
> > > 
> > > Are you sure this is set up right?
> > 
> > This is actually (confusingly) correct.
> > 
> > Checking the code we simply link to libc, not to anything specific in
> > gettext-devel.
> >
> 
> Weird, the readme seems to imply that it needs to link to gettext-devel...

Ah, all is clear now. Look at the version I'm working on vs upstream master

https://github.com/gosexy/gettext/commits/98b7b91596d20b96909e6b60d57411547dd9959c

I guess I should simply update the package to take the most recent release/commit.

Thanks
ZK

Comment 6 Neal Gompa 2016-07-25 20:48:29 UTC
It looks like there's quite a few changes between that commit and master: https://github.com/gosexy/gettext/compare/98b7b91596d20b96909e6b60d57411547dd9959c...master

There may be other new dependencies involved here...

Comment 7 Zygmunt Krynicki 2016-07-26 08:31:45 UTC
I've updated the package to latest upstream snapshot. I also made unit tests to run and pass.

The SRPM is now golang-github-gosexy-gettext-0-0.2.git305f360.fc24.src.rpm (the spec file name is the same). Both were pushed to my fedora people website.

Comment 8 Neal Gompa 2016-07-26 09:35:26 UTC
In the future, please post a comment with the Spec URL and an updated SRPM URL when you update them. It makes it easier for folks reviewing on a mobile device or by email.

Shouldn't the go-flags import be declared, too? It seems to be pulled in at https://github.com/gosexy/gettext/blob/master/go-xgettext/main.go#L42

Comment 9 Zygmunt Krynicki 2016-08-09 11:45:03 UTC
I fixed the missing build dependency on go-flags and started building the go-xgettext binary in a separate binary package. Built-tested in mock.

Spec file: https://fedorapeople.org/~zyga/golang-github-gosexy-gettext.spec
SRPM: https://fedorapeople.org/~zyga/golang-github-gosexy-gettext-0-0.2.git305f360.fc24.src.rpm

Comment 10 Neal Gompa 2016-08-09 11:49:17 UTC
Package was generated through gofed, simplifying the review considerably.

- Conforms to packaging guidelines (gofed generated spec)
- license correct and valid
- only sources installed in all subpackages except go-xgettext, which provides /usr/bin/go-xgettext

Package approved.

Comment 11 Gwyn Ciesla 2016-08-09 13:06:32 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/golang-github-gosexy-gettext

Comment 12 Fedora Update System 2016-08-09 13:55:43 UTC
golang-github-gosexy-gettext-0-0.2.git305f360.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2016-6abbfaa875

Comment 13 Fedora Update System 2016-08-09 14:13:38 UTC
golang-github-gosexy-gettext-0-0.2.git305f360.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-5b459e3543

Comment 14 Fedora Update System 2016-08-09 14:23:33 UTC
golang-github-gosexy-gettext-0-0.2.git305f360.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2016-0eff5c48e4

Comment 15 Fedora Update System 2016-08-09 15:25:11 UTC
golang-github-gosexy-gettext-0-0.2.git305f360.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-5b459e3543

Comment 16 Fedora Update System 2016-08-09 22:22:40 UTC
golang-github-gosexy-gettext-0-0.2.git305f360.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-6abbfaa875

Comment 17 Fedora Update System 2016-08-10 02:50:14 UTC
golang-github-gosexy-gettext-0-0.2.git305f360.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-0eff5c48e4

Comment 18 Fedora Update System 2016-08-18 00:49:54 UTC
golang-github-gosexy-gettext-0-0.2.git305f360.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.

Comment 19 Fedora Update System 2016-08-18 03:51:04 UTC
golang-github-gosexy-gettext-0-0.2.git305f360.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report.

Comment 20 Fedora Update System 2016-08-27 10:45:20 UTC
golang-github-gosexy-gettext-0-0.2.git305f360.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report.


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