[Bug 1212124] Review Request: golang-github-ryanuber-columnize- Easy column formatted output for golang

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

 



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



--- Comment #2 from Jan Chaloupka <jchaloup@xxxxxxxxxx> ---
> This is an unofficial review.

Thanks Alexander. Any review is welcomed.

> For such a small source package, I did not expect such an elaborate spec
> file, I was in awe when I opened it. Obviously, you've been doing this
> for quite some time. The spec file itself is up to speed with the latest
> versions of rpm - no deprecated or unnecessary commands or macros.

Such a small package consisting of one go package and one test file is rare.
Usually it has more packages. Thanks, we have been trying to use as few
commands as possible.

> I don't speak Go and from what I read on wikipedia about the language's
> structure and from https://fedoraproject.org/wiki/Packaging:Debuginfo
> I am not sure why a debuginfo package is not needed (perhaps it falls
> under the "Useless or incomplete debuginfo packages due to other reasons"
> clause?).

Debuginfo packages are provided only for subpackages providing binaries. As
this package provides only source codes there is nothing to debug and thus
debuginfo is empty

> On the other hand -and perhaps I should be asking this on the devel ML- 
> don't understand the need for a separate devel package. This is clearly
> a utility to be used in other packages/programs (not sure about the Go
> terminology) and what goes in the main package goes into the devel package

What line? This package has no %files section, only %files devel.

> (should the test be there?) as well.

All tests are in %check section. Sometimes the section is empty as some
additional packages are needed for testing. Tests are optional at the moment.

> Can there be -devel only packages?
> I'd really appreciate it if you could explain this to me.

As the main purpose of almost all golang packages is "being buildtime
dependency", there is usually only devel subpackage. And as there is a high
change a golang project is going to be used in other golang projects, each spec
file provides at least one devel subpackage. You can see devel subpackage as
static subpackage of C project. However, as there is nothing as shared library
in golang we ship all golang source codes. Once a golang project is built (go
build) we have binaries as well. Binaries and source codes are separeted. Check
etcd or kubernetes packages for example.

> According to https://fedoraproject.org/wiki/PackagingDrafts/Go there is a
> %{go_arches} macro, perhaps you could use it instead of %{ix86} x86_64 %{arm}
> in your ExclusiveArch, but then again it applies to older versions of fedora > and I don't remember what was supported at the time.

There was a problem with %{go_arches} it was not available in epel6. IIRC
scratch build was ok but raw build failed on missing %{go_arches}.

> The warning you get from rpmlint is because you actually have
> "Summary:       \tEasy column formatted output for golang "
> I opened the file in Writer to look for the tab stop, if you delete the last > whitespace character before the "E" and hit the space bar, you'll get rid of > the warning.

Thanks, I usually skip those warnings because once I update a spec file to the
latest commit, sometimes patch is needed and sometimes i add space sometimes
tab. It is fixed now.

> I'm still struggling with git, but I think that the closest upstream release > version to commit 44cb478 is v2.0.1, not 0.1. Did I get this right?

Yes, you are right. Spec files are generated via gofed [1] and it does not
check how many releases a project has. This has to be done manually and I am
lazy about it as checking it for each project you get mad. This is one of 21
new golang packages to get to Fedora. I can try to detect it via git tags but I
does not have to be 1:1 mapping (the latest tag does not has to be the latest
release :(). I fixed this too, thanks.

[1] https://github.com/ingvagabund/gofed

-- 
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
https://admin.fedoraproject.org/mailman/listinfo/package-review





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