[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

Alexander Ploumistos <alex.ploumistos@xxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |alex.ploumistos@xxxxxxxxx



--- Comment #1 from Alexander Ploumistos <alex.ploumistos@xxxxxxxxx> ---
Hello,

This is an unofficial review. 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.

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?).

On the other hand -and perhaps I should be asking this on the devel ML- I 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 (should the test be
there?) as well. Can there be -devel only packages? I'd really appreciate it if
you could explain this to me.

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.

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.

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?

I have verified all the items that were checked by fedora-review, I'll just
comment below on the items that needed manual review:

===== MUST items =====

Generic:
[ ]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.

     Yes, MIT license.

[ ]: License field in the package spec file matches the actual license.

     It does.

[ ]: Package contains no bundled libraries without FPC exception.

     No bundled libraries present.

[ ]: Changelog in prescribed format.

     Just one entry, properly formatted.

[ ]: Sources contain only permissible code or content.

     They do.

[ ]: Package contains desktop file if it is a GUI application.

     It doesn't, as it is not a GUI application.

[ ]: Development files must be in a -devel package

     They are, but I am confused as to why and if the test should be there...

[ ]: Package uses nothing in %doc for runtime.

     No such thing.

[ ]: Package consistently uses macros (instead of hard-coded directory names).

     All paths appear consistent to what the Go packaging draft specifies. 

[ ]: Package is named according to the Package Naming Guidelines.

     Consistent with the guidelines about snapshot packages (but perhaps it
should be 2.0.1?).

[ ]: Package does not generate any conflict.

     No conflicts encountered in the mock build.

[ ]: Package obeys FHS, except libexecdir and /usr/target.

     It does.

[ ]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.

     Does not apply.

[ ]: Requires correct, justified where necessary.

     Not sure about the version requirement, unless I misunderstood something,
upstream built the package successfully with a lower Go version.

[ ]: Spec file is legible and written in American English.

     Spec file is beautiful and in English. Didn't notice any biscuits,
barristers or lorries :)

[ ]: Package contains systemd file(s) if in need.

     Not needed.

[ ]: Package is not known to require an ExcludeArch tag.

     ExclusiveArch is in place for fedora < 19 and rhel < 7.

[ ]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 10240 bytes in 2 files.

     Doesn't seem to justify a separate package.

[ ]: Package complies to the Packaging Guidelines

     It does.


===== SHOULD items =====

Generic:
[ ]: If the source package does not include license text(s) as a separate file
     from upstream, the packager SHOULD query upstream to include it.

     Upstream provides such a file.

[ ]: Final provides and requires are sane (see attachments).

     They are and they appear structured as required by the Go packaging draft.

[ ]: Package functions as described.

     I'm no expert, but from a cursory glance at the source code, it should.

[ ]: Latest version is packaged.

     It is.

[ ]: Package does not include license text files separate from upstream.

     It does not.

[ ]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.

     Not applicable.

[ ]: Package should compile and build into binary rpms on all supported
     architectures.

     Package is noarch.

[ ]: %check is present and all tests pass.

     Both true.

[ ]: Packages should try to preserve timestamps of original installed files.

     Package invokes install and cp with the -p flag.

-- 
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]