[Bug 1255179] Review Request: runc - CLI for running Open Containers

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

 



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



--- Comment #15 from Jan Chaloupka <jchaloup@xxxxxxxxxx> ---
> [!]: Package requires other packages for directories it uses.
> Subdirectories under /usr/share/gocode that aren't owned by golang or the
> opencontainer-specs package should be owned by this package so that its 
> removal doesn't leave empties around.

Spec file updated. All directories are owned by devel subpackage. As user could
update a devel subpackage without updating unit-test, both packages could end
up owning the same directory. So no directories owned by unit-test.

> [ ]: %build honors applicable compiler flags or justifies otherwise.
>  Might want to add a comment about the -B flag here, for people like me who 
> didn't know that it tells the go linker to embed a build ID note in the
> binary.

Described in Packaging Draft [1].

> [ ]: Changelog in prescribed format.
>  It's customary to use a person's full name where the changelog currently
> lists a Fedora account name.  It's not a blocker, but my guess is it's an
> oversight.

Updated.

> [ ]: Requires correct, justified where necessary.
>  The runc-devel package appears to be missing a requires on 
> "golang(github.com/opencontainers/specs)", which is imported by multiple
> parts of libcontainer.

$ gofed ggi --all-occurrences --show-occurrence | grep
github.com/opencontainers/specs
github.com/opencontainers/specs (spec.go:main, restore.go:main, utils.go:main,
run.go:main)

github.com/opencontainers/specs is used only in main packages. These are not to
be imported by other packages.

> [ ]: Package complies to the Packaging Guidelines
> The runc command should (eventually) have a manual page.

Definitely. 

> The package description should be more than just a copy of the summary.

At the moment I don't see any description that would be explaining enough what
runc is. If you have an idea what description is suitable, please. I will
update the spec file accordingly. 

> [ ]: Package successfully compiles and builds into binary rpms on at least
>     one supported primary architecture.
> Missing BuildRequires on "golang(github.com/opencontainers/specs)" (from bug
> #1255370) and "golang(github.com/codegangsta/cli)".

Updated. Thanks. This is not included in the spec file generator. I will update
the generator as well.

> [ ]: Fully versioned dependency in subpackages if applicable.
> Whether runc-devel or runc-unit-test should depend on the same version of
> runc isn't really clear to me, since there aren't any dangling symlinks if
> they don't.
>     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in 
>     runc-devel, runc-unit-test

Neither devel nor unit-test depends on runc.

> [!]: Latest version is packaged.

By the time of generating the spec file, the commit was the latest one.

> [ ]: Spec use %global instead of %define unless justified.
>  Why is copying() defined using %define rather than %global?

Because copying is a parametric macro which needs to be evaluated in the time
of use. As a global, it would be evaluated in the time of definition end up
with empty %license macro.

[1] https://fedoraproject.org/wiki/PackagingDrafts/Go
=========================

Spec URL: https://jchaloup.fedorapeople.org/reviews/runc/runc.spec

SRPM URL:
https://jchaloup.fedorapeople.org/reviews/runc/runc-0.2-0.2.git90e6d37.fc20.src.rpm

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