[Bug 1411962] Review Request: golang-github-milochristiansen-lua - A Lua 5.3 VM and compiler written in Go

[bugzilla]

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



--- Comment #22 from Fedora Update System  ---
golang-github-milochristiansen-lua-1.1.3-1.fc24 has been pushed to the Fedora
24 stable repository. If problems still persist, please make note of it in this
bug report.

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1411962] Review Request: golang-github-milochristiansen-lua - A Lua 5.3 VM and compiler written in Go

[bugzilla]

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

Fedora Update System  changed:

   What|Removed |Added

 Status|ON_QA   |CLOSED
 Resolution|--- |ERRATA
Last Closed|2017-05-04 09:29:45 |2017-05-15 00:34:54



--- Comment #21 from Fedora Update System  ---
golang-github-milochristiansen-lua-1.1.3-1.fc25 has been pushed to the Fedora
25 stable repository. If problems still persist, please make note of it in this
bug report.

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1411962] Review Request: golang-github-milochristiansen-lua - A Lua 5.3 VM and compiler written in Go

[bugzilla]

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



--- Comment #20 from Fedora Update System  ---
golang-github-milochristiansen-lua-1.1.3-1.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-2017-6dd45a7b01

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1411962] Review Request: golang-github-milochristiansen-lua - A Lua 5.3 VM and compiler written in Go

[bugzilla]

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

Fedora Update System  changed:

   What|Removed |Added

 Status|CLOSED  |ON_QA
 Resolution|ERRATA  |---
   Keywords||Reopened



--- Comment #19 from Fedora Update System  ---
golang-github-milochristiansen-lua-1.1.3-1.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-2017-17e1e108f9

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1411962] Review Request: golang-github-milochristiansen-lua - A Lua 5.3 VM and compiler written in Go

[bugzilla]

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

Fedora Update System  changed:

   What|Removed |Added

 Status|MODIFIED|CLOSED
 Resolution|--- |ERRATA
Last Closed||2017-05-04 09:29:45



--- Comment #18 from Fedora Update System  ---
golang-github-milochristiansen-lua-1.1.3-1.fc26 has been pushed to the Fedora
26 stable repository. If problems still persist, please make note of it in this
bug report.

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1411962] Review Request: golang-github-milochristiansen-lua - A Lua 5.3 VM and compiler written in Go

[bugzilla]

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



--- Comment #17 from Fedora Update System  ---
golang-github-milochristiansen-lua-1.1.3-1.fc24 has been submitted as an update
to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2017-17e1e108f9

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1411962] Review Request: golang-github-milochristiansen-lua - A Lua 5.3 VM and compiler written in Go

[bugzilla]

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

Fedora Update System  changed:

   What|Removed |Added

 Status|ON_QA   |MODIFIED



-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1411962] Review Request: golang-github-milochristiansen-lua - A Lua 5.3 VM and compiler written in Go

[bugzilla]

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



--- Comment #16 from Fedora Update System  ---
golang-github-milochristiansen-lua-1.1.3-1.fc25 has been submitted as an update
to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2017-6dd45a7b01

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1411962] Review Request: golang-github-milochristiansen-lua - A Lua 5.3 VM and compiler written in Go

[bugzilla]

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

Fedora Update System  changed:

   What|Removed |Added

 Status|MODIFIED|ON_QA



--- Comment #15 from Fedora Update System  ---
golang-github-milochristiansen-lua-1.1.3-1.fc26 has been pushed to the Fedora
26 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-2017-e960574979

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1411962] Review Request: golang-github-milochristiansen-lua - A Lua 5.3 VM and compiler written in Go

[bugzilla]

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



--- Comment #14 from Fedora Update System  ---
golang-github-milochristiansen-lua-1.1.3-1.fc26 has been submitted as an update
to Fedora 26. https://bodhi.fedoraproject.org/updates/FEDORA-2017-e960574979

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1411962] Review Request: golang-github-milochristiansen-lua - A Lua 5.3 VM and compiler written in Go

[bugzilla]

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

Fedora Update System  changed:

   What|Removed |Added

 Status|NEW |MODIFIED



-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1411962] Review Request: golang-github-milochristiansen-lua - A Lua 5.3 VM and compiler written in Go

[bugzilla]

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



--- Comment #13 from Ben Rosser  ---
Sure, I'll fix that on import. 

Thanks for the reviews (and the golang packaging advice)!

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1411962] Review Request: golang-github-milochristiansen-lua - A Lua 5.3 VM and compiler written in Go

[bugzilla]

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



--- Comment #12 from Gwyn Ciesla  ---
Package request has been approved:
https://admin.fedoraproject.org/pkgdb/package/rpms/golang-github-milochristiansen-lua

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1411962] Review Request: golang-github-milochristiansen-lua - A Lua 5.3 VM and compiler written in Go

[bugzilla]

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

Fabio Valentini  changed:

   What|Removed |Added

  Flags|fedora-review?  |fedora-review+



--- Comment #11 from Fabio Valentini  ---
It's nice that is was possible to resolve the bug upstream!
The tests pass now:
https://koji.fedoraproject.org/koji/taskinfo?taskID=19065240

My final review uncovered one last minor issue:


 Note: No known owner of
 /usr/share/gocode/src/github.com/milochristiansen/lua/vendor



You should add this as a %dir to the -devel subpackage before importing the
package (no additional changelog entry is necessary from my point of view).

For example, add something like the following at line 104 (similar to line
103):

echo "%%dir %%{gopath}/src/%%{import_path}/vendor" >> devel.file-list


But since it's only a very minor issue, the package is approved.

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1411962] Review Request: golang-github-milochristiansen-lua - A Lua 5.3 VM and compiler written in Go

[bugzilla]

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



--- Comment #10 from Ben Rosser  ---
Indeed; it turned out to be an integer conversion issue. The bug has now been
fixed in 1.1.3:

Spec URL:
https://tc01.fedorapeople.org/dwarffortress/golang-github-milochristiansen-lua.spec
SRPM URL:
https://tc01.fedorapeople.org/dwarffortress/golang-github-milochristiansen-lua-1.1.3-1.fc25.src.rpm

I've also added the new provides, the supermeta tests, and the
commit/shortcommit macros to the header.

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1411962] Review Request: golang-github-milochristiansen-lua - A Lua 5.3 VM and compiler written in Go

[bugzilla]

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



--- Comment #9 from Fabio Valentini  ---
Let's see how upstream responds. I had similar problems with some of my go
packages, most of the time the issue was an integer overflow (caused by test
cases, not by code) on 32bit arches that developers didn't think of.

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1411962] Review Request: golang-github-milochristiansen-lua - A Lua 5.3 VM and compiler written in Go

[bugzilla]

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



--- Comment #8 from Ben Rosser  ---
> PS: I usually re-run gofed when I bump my golang packages to a new version to 
> make sure I catch issues like [1] and [2]. Maybe that helps you in the future!

Thanks! This is a good suggestion; I'll be sure to bear it in mind.

> It seems tests (and builds) fail on 32 bit architectures:
https://koji.fedoraproject.org/koji/taskinfo?taskID=18810051

> You should inform upstream about that.

I have done so here: https://github.com/milochristiansen/lua/issues/7. In the
mean time, should I simply modify the ExclusiveArch tag to not build on the
32-bit architectures? (What would be a good way to do that, since it uses the
go_arches macro?).

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1411962] Review Request: golang-github-milochristiansen-lua - A Lua 5.3 VM and compiler written in Go

[bugzilla]

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



--- Comment #7 from Fabio Valentini  ---
It seems tests (and builds) fail on 32 bit architectures:
https://koji.fedoraproject.org/koji/taskinfo?taskID=18810051

You should inform upstream about that.

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1411962] Review Request: golang-github-milochristiansen-lua - A Lua 5.3 VM and compiler written in Go

[bugzilla]

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



--- Comment #6 from Fabio Valentini  ---
1) The new release provides a few new go subpackages:

> Provides:  golang(%{import_path}/supermeta) = %{version}-%{release}
> Provides:  golang(%{import_path}/testhelp) = %{version}-%{release}

Please add them to the -devel subpackage's Provides: section.


2) Additionally, add

> %gotest %{import_path}/supermeta

after the

> %gotest %{import_path}

line, since that subpackage also has tests.


3) Please add the

> %global commit  cbee393f691f87139e9f21525610767e85ae0e33
> %global shortcommit %(c=%{commit}; echo ${c:0:7})

and maybe

> # commit cbee393f691f87139e9f21525610767e85ae0e33 == version 1.1.1

to the .spec header (after the import_path definition, where you removed it
from in the first place). This information is used by gofed to track versions
of golang packages across all of fedora.


Otherwise, the package looks good now!


PS: I usually re-run gofed when I bump my golang packages to a new version to
make sure I catch issues like [1] and [2]. Maybe that helps you in the future!

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1411962] Review Request: golang-github-milochristiansen-lua - A Lua 5.3 VM and compiler written in Go

[bugzilla]

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



--- Comment #5 from Ben Rosser  ---
Updated to upstream release 1.1.1* and fixed the following:

- Updated to latest upstream release.
- Changed Source0 to use project prefix.
- Remove vendored go library from Provides section.
- Cleaned up setup macro in prep section.
- Removed some empty/redundant template parts of the spec file and cleaned
GOPATH definition.
- General spec cleanup; removed dist tag from changelog section and removed
leading article from summary.

Spec URL:
https://tc01.fedorapeople.org/dwarffortress/golang-github-milochristiansen-lua.spec
SRPM URL:
https://tc01.fedorapeople.org/dwarffortress/golang-github-milochristiansen-lua-1.1.1-1.fc25.src.rpm

*There is a more recent commit referring to 1.1.2, but that release hasn't been
tagged.

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1411962] Review Request: golang-github-milochristiansen-lua - A Lua 5.3 VM and compiler written in Go

[bugzilla]

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



--- Comment #4 from Ben Rosser  ---
> 4) Are you sure it's a good idea to provide the "vendored" go library? If 
> not, just remove the "Provides: golang(%{import_path}/vendor/sliceutil) = 
> %{version}-%{release}".

I wasn't sure if I should do so or not; but I can remove this from the spec
when I update it with all the other comments.

I'll post an updated version of the spec later today, sorry for the delay.

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1411962] Review Request: golang-github-milochristiansen-lua - A Lua 5.3 VM and compiler written in Go

[bugzilla]

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



--- Comment #3 from Fabio Valentini  ---
Pinging submitter.

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1411962] Review Request: golang-github-milochristiansen-lua - A Lua 5.3 VM and compiler written in Go

[bugzilla]

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



--- Comment #2 from Fabio Valentini  ---
Initial comments:

1) Please update your .spec and .src.rpm for the newest upstream version
(1.1.0).

2) Please drop the leading "A" from the Summary line.

3) Please change the target file name for your sources, for example you can
use: "Source0:
https://%{provider_prefix}/archive/v%{version}/%{project}-%{repo}-%{version}.tar.gz;
to avoid name collisions with "official" lua package sources (otherwise your
sources will be "lua-1.1.0.tar.gz", which looks like official lua sources ...).

4) Are you sure it's a good idea to provide the "vendored" go library? If not,
just remove the "Provides: golang(%{import_path}/vendor/sliceutil) =
%{version}-%{release}".

5) You can remove the empty %if-endif block at line 77.

6) You can use "%setup -q -n %{repo}-%{version}" in %prep.

7) Please correct the GOPATH definition on line 136, the Godeps directory isn't
in the sources, so this seems to be there by accident.

8) Please drop the %{dist} tag from the changelog entry, it doesn't belong
there.

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1411962] Review Request: golang-github-milochristiansen-lua - A Lua 5.3 VM and compiler written in Go

[bugzilla]

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

Fabio Valentini  changed:

   What|Removed |Added

 CC||decatho...@gmail.com
   Assignee|nob...@fedoraproject.org|decatho...@gmail.com
  Flags||fedora-review?



--- Comment #1 from Fabio Valentini  ---
Taking this review.

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org