[Bug 1451124] (cni) Review Request: containernetworking - CNI Plugins

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

 



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



--- Comment #1 from Jan Chaloupka <jchaloup@xxxxxxxxxx> ---
Given the cni is a go project:
- if it is possible we should package entire devel subpackage (including its
dependencies)
- if it is possible the project should be built from bundled dependencies
- if possible, project tests should be run
- if possible, the project should be built over all available architectures
- if possible, the spec file header should contain macros that reference
project location

I have generated the new spec file via gofed and incorporated part of your spec
file into it [1].

Currently (referring to your spec file):
- project is built from bundled dependencies (I would recommended that only in
cases when there is no other way)
- devel subpackage is missing build-time and run-time dependencies
- tests are not run (given some of the tests need to be run as a root and
others need additional configuration, they do not have to be run)
- no unit-test subpackage: even if the tests are not run, the _test.go files
can by analysed by tooling and/or run in specialized CI so it is always useful
to package the unit-test package as well
- though there is no strict structure of the spec file header, I recommend to
define provider_prefix, import_path and commit macros. I got a tooling that
periodically scans all go projects in Fedora rawhide and collects various
data/metadata from the go rpms. Plus, if the macros are set, you can use gofed
tooling to update the spec file.
- exclusive archs are set to %{go_arches}: out aim is to generate spec files
that can be built over various architectures and OSes. Unfortunately, the
%{go_arches} macro is not present on CentOS or RHEL so the current spec file
with work on Fedora only. Check [1] for the general ExclusiveArch value. The
same holds for golang BuildRequires.
- in Fedora %gobuild macro is defined that encompasses the BUILD_ID for you so
there is no need to do that manually. However, the same limitations hold here.
The macro is present on Fedora only so the macro is redefine at spec file
header if not defined, see [1].
- I would not call the ./build script as it usually runs more than is needed
and hides the building itself. As long as the build steps are simple, I would
recommend to put them into the %build section.
- not sure about the name, kubernetes uses kubernetes-cni, which I would see
instead of containernetworking. But, I have no strong preference over the name
so basically any name that is close to cni is acceptable.

The references spec file [1] is an example, not something you need to follow.
Yeah, it is pretty long and there is a lot of constructions that make it hard
to read. But, it has its advantages.

[1] https://github.com/gofed/reviews/pull/5/files

-- 
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
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx




[Index of Archives]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite Conditions]     [KDE Users]

  Powered by Linux