Bug 1359802
Summary: | Review Request: golang-github-gosexy-gettext - Gettext support for the Go language | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Zygmunt Krynicki <me> |
Component: | Package Review | Assignee: | Neal Gompa <ngompa13> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | me, ngompa13, package-review |
Target Milestone: | --- | Flags: | ngompa13:
fedora-review+
|
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | If docs needed, set a value | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2016-08-18 00:49:56 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: |
Description
Zygmunt Krynicki
2016-07-25 13:17:49 UTC
Taking this review. 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? (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 (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 (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 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... 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. 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 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 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. Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/golang-github-gosexy-gettext 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 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 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 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 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 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 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. 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. 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. |