Bug 1677668
| Summary: | Review Request: coreos-installer - Installer for CoreOS systems | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Dusty Mabe <dustymabe> |
| Component: | Package Review | Assignee: | Neal Gompa <ngompa13> |
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | ngompa13, package-review |
| Target Milestone: | --- | Flags: | ngompa13:
fedora-review+
|
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | If docs needed, set a value | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2019-02-26 03:05:03 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
|
Description
Dusty Mabe
2019-02-15 14:22:48 UTC
Taking this review. So, here's a first pass... > %global provider github > %global provider_tld com > %global project coreos > %global repo coreos-installer > # https://github.com/coreos/coreos-installer > %global provider_prefix %{provider}.%{provider_tld}/%{project}/%{repo} > %global import_path %{provider_prefix} > %global commit 081d4bed42489a48e95f559022d96f4999e56cbd > %global shortcommit %(c=%{commit}; echo ${c:0:7}) Holy crap, this is so much overkill. You seem to only need %commit and %shortcommit. All the rest could be flattened. > URL: https://%{provider_prefix} > Source0: https://%{provider_prefix}/archive/%{commit}/%{repo}-%{shortcommit}.tar.gz This can be simplified to the following: URL: https://github.com/coreos/%{name} Source0: %{url}/archive/%{commit}/%{name}-%{shortcommit}.tar.gz > # setup command reference: http://ftp.rpm.org/max-rpm/s1-rpm-inside-macros.html > # unpack source0 and apply patches > %setup -T -b 0 -q -n %{repo}-%{commit} This can be simplified to just "%autosetup -n %{name}-%{commit} -p1" > Requires: %{name} = %{version}-%{release} > Requires: coreos-installer You already have "Requires: %{name} = %{version}-%{release}", you don't need "Requires: coreos-installer" too. The second "BuildArch: noarch" is redundant. yeah there was a bit of overkill. I liked the structure so I copied it from another spec file. Addressed all the comments: https://github.com/dustymabe/coreos-installer-rpm/raw/1eec556ad36e7325659f80bc2cf7f84af9adfb18/coreos-installer.spec https://github.com/dustymabe/coreos-installer-rpm/raw/1eec556ad36e7325659f80bc2cf7f84af9adfb18/coreos-installer-0-1.git081d4be.fc29.src.rpm You can see the diff in the spec file here: https://github.com/dustymabe/coreos-installer-rpm/commit/1eec556ad36e7325659f80bc2cf7f84af9adfb18 Review notes: [x]: Package follows Fedora package naming guidelines [x]: Package builds and installs correctly [x]: Package follows licensing guidelines and installs license content properly [x]: Package follows packaging guidelines PACKAGE APPROVED. (fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/coreos-installer coreos-installer-0-1.git081d4be.fc29 has been submitted as an update to Fedora 29. https://bodhi.fedoraproject.org/updates/FEDORA-2019-b71e05bad3 coreos-installer-0-1.git081d4be.fc29 has been pushed to the Fedora 29 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-2019-b71e05bad3 coreos-installer-0-1.git081d4be.fc29 has been pushed to the Fedora 29 stable repository. If problems still persist, please make note of it in this bug report. |