Bug 1492866 - Review Request: exercism - Exercism command-line interface
Summary: Review Request: exercism - Exercism command-line interface
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Robert-André Mauchin 🐧
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2017-09-18 19:18 UTC by Clément DAVID
Modified: 2017-10-23 04:43 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2017-09-22 15:55:15 UTC
Type: ---
Embargoed:
zebob.m: fedora-review+


Attachments (Terms of Use)

Description Clément DAVID 2017-09-18 19:18:29 UTC
Spec URL: https://davidcl.fedorapeople.org/exercism/exercism.spec
SRPM URL: https://davidcl.fedorapeople.org/exercism/exercism-2.4.1-1.fc26.src.rpm
Description: Exercism provides a way to do the problems on https://exercism.io
Fedora Account System Username: davidcl

Comment 1 Robert-André Mauchin 🐧 2017-09-19 09:01:48 UTC
Hello,

 - In the description, https://exercism.io should not be https but http: the site is not available in https.

 - I've got a build error:

+ go build -o out/exercism exercism/main.go
go: GOPATH entry is relative; must be absolute path: "".
For more details see: 'go help gopath'

  To solve this, I've modified your SPEC like this:

%prep
%setup -q -n %{repo}-%{version}
%patch1 -p1

%build
mkdir -p ./_build/src/%{provider}.%{provider_tld}/%{project}
ln -s $(pwd) ./_build/src/%{provider}.%{provider_tld}/%{project}/cli
export GOPATH=$(pwd)/_build:%{gopath}
go build -o out/exercism exercism/main.go

  - Packaging -devel and -unit-test-devel are not needed for a Go binary, just keep the main package. Thus you should also remove the Provides: only needed if the package is used as a library.

Comment 2 Clément DAVID 2017-09-20 06:05:00 UTC
(In reply to Robert-André Mauchin from comment #1)
>  - In the description, https://exercism.io should not be https but http: the
> site is not available in https.

Nice catch ! I added the s blindly.
 
>  - I've got a build error:
> 
> + go build -o out/exercism exercism/main.go
> go: GOPATH entry is relative; must be absolute path: "".
> For more details see: 'go help gopath'
>   To solve this, I've modified your SPEC like this:
> 
> %prep
> %setup -q -n %{repo}-%{version}
> %patch1 -p1
> 
> %build
> mkdir -p ./_build/src/%{provider}.%{provider_tld}/%{project}
> ln -s $(pwd) ./_build/src/%{provider}.%{provider_tld}/%{project}/cli
> export GOPATH=$(pwd)/_build:%{gopath}
> go build -o out/exercism exercism/main.go

After some investigation, I'm able to reproduce. The 'GOPATH' did not expand to anything on mock (and on your system). A trailing "" is then resolved as a relative path. FIXED

>   - Packaging -devel and -unit-test-devel are not needed for a Go binary,
> just keep the main package. Thus you should also remove the Provides: only
> needed if the package is used as a library.

OK, I removed them then and cleaned all the spec.

Comment 3 Clément DAVID 2017-09-20 06:05:43 UTC
Spec URL: https://davidcl.fedorapeople.org/exercism/exercism.spec
SRPM URL: https://davidcl.fedorapeople.org/exercism/exercism-2.4.1-1.fc26.src.rpm
Description: Exercism provides a way to do the problems on https://exercism.io
Fedora Account System Username: davidcl

Comment 4 Robert-André Mauchin 🐧 2017-09-20 06:13:35 UTC
You didn't remove:

%if 0%{?with_devel}
%files devel -f devel.file-list
%license LICENSE
%doc CHANGELOG.md RELEASE.md README.md
%dir %{gopath}/src/%{provider}.%{provider_tld}/%{project}
%endif

%if 0%{?with_unit_test} && 0%{?with_devel}
%files unit-test-devel -f unit-test-devel.file-list
%license LICENSE
%doc CHANGELOG.md RELEASE.md README.md
%endif


??

Comment 5 Clément DAVID 2017-09-21 18:34:58 UTC
(In reply to Robert-André Mauchin from comment #4)
> You didn't remove:
> 
> %if 0%{?with_devel}
> %files devel -f devel.file-list
> %license LICENSE
> %doc CHANGELOG.md RELEASE.md README.md
> %dir %{gopath}/src/%{provider}.%{provider_tld}/%{project}
> %endif
> 
> %if 0%{?with_unit_test} && 0%{?with_devel}
> %files unit-test-devel -f unit-test-devel.file-list
> %license LICENSE
> %doc CHANGELOG.md RELEASE.md README.md
> %endif
> 
> 
> ??

Right that's a miss ! Updated at :

Spec URL: https://davidcl.fedorapeople.org/exercism/exercism.spec
SRPM URL: https://davidcl.fedorapeople.org/exercism/exercism-2.4.1-1.fc26.src.rpm
Description: Exercism provides a way to do the problems on https://exercism.io
Fedora Account System Username: davidcl

Comment 6 Robert-André Mauchin 🐧 2017-09-21 18:50:32 UTC
Ok for me, package accepted.

Comment 7 Gwyn Ciesla 2017-09-22 12:45:58 UTC
(fedrepo-req-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/exercism

Comment 8 Clément DAVID 2017-09-22 15:55:15 UTC
build completed http://koji.fedoraproject.org/koji/buildinfo?buildID=974100

Comment 9 Elliott Sales de Andrade 2017-10-23 04:43:19 UTC
So I accidentally tried to package this again before seeing this review. Are you planning to build for 27 or 26?

Also, I think half the description is a bit irrelevant. A Fedora user doesn't care too much that there are no "extra" language dependencies, because that's all handled by the package manager. I went with something like this:

Exercism allows you to download and solve practice problems in over 30
different languages. Exercism takes place in two places: the discussions happen
on the website, and you work on exercises locally. The Command Line Interface
bridges the gap, allowing you to fetch exercises and submit solutions to the
site.


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