Bug 1677668 - Review Request: coreos-installer - Installer for CoreOS systems
Summary: Review Request: coreos-installer - Installer for CoreOS systems
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Neal Gompa
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2019-02-15 14:22 UTC by Dusty Mabe
Modified: 2019-02-26 03:05 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2019-02-26 03:05:03 UTC
Type: ---
Embargoed:
ngompa13: fedora-review+


Attachments (Terms of Use)

Description Dusty Mabe 2019-02-15 14:22:48 UTC
Spec URL: https://raw.githubusercontent.com/dustymabe/coreos-installer-rpm/master/coreos-installer.spec
SRPM URL: https://github.com/dustymabe/coreos-installer-rpm/raw/master/coreos-installer-0-1.git081d4be.fc29.src.rpm
Fedora Account System Username: dustymabe

Description:
This package contains the coreos-installer script used to install CoreOS                                                                                        
disk images to bare metal machines.

Comment 1 Neal Gompa 2019-02-15 14:44:51 UTC
Taking this review.

Comment 2 Neal Gompa 2019-02-15 14:54:50 UTC
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.

Comment 4 Neal Gompa 2019-02-15 15:17:23 UTC
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.

Comment 5 Mohan Boddu 2019-02-15 17:24:42 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/coreos-installer

Comment 6 Fedora Update System 2019-02-15 23:09:36 UTC
coreos-installer-0-1.git081d4be.fc29 has been submitted as an update to Fedora 29. https://bodhi.fedoraproject.org/updates/FEDORA-2019-b71e05bad3

Comment 7 Fedora Update System 2019-02-16 03:05:22 UTC
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

Comment 8 Fedora Update System 2019-02-26 03:05:03 UTC
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.


Note You need to log in before you can comment on or make changes to this bug.