Bug 2355279 - Review Request: rubygem-tcxread - tcx reader/parser in Ruby
Summary: Review Request: rubygem-tcxread - tcx reader/parser in Ruby
Keywords:
Status: NEW
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL: https://github.com/firefly-cpp/tcxread
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2025-03-27 10:26 UTC by Benson Muite
Modified: 2025-04-06 12:06 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed:
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Benson Muite 2025-03-27 10:26:26 UTC
spec: https://fed500.fedorapeople.org/rubygem-tcxread.spec
srpm: https://fed500.fedorapeople.org/rubygem-tcxread-0.2.0-1.fc43.src.rpm

description:
tcx reader/parser in Ruby.

fas: fed500

Reproducible: Always

Comment 1 Vít Ondruch 2025-04-02 08:46:35 UTC
A few comments.

1) If you keep something like `#Source: https://rubygems.org/gems/%%{gem_name}-%%{version}.gem`, you can try to go with `%dnl`. I.e. `%dnl Source: https://rubygems.org/gems/%{gem_name}-%{version}.gem`. That way, you don't have to escape the `%`.

2) What is the reason to go with the upstream tarball anyway? Is it the test? Isn't it better to include just the test separately?

3) I'd suggest to improve the `description`. I see it comes from the .gemspec like that, but we don't need to stick with that. The initial paragraph from the README would be likely much better description.

4) We typically execute the test suite under `.%{gem_instdir}`. And this actually might be related to (2). The reason is that the content in `.%{gem_instdir}` is what is used in `%install` section. If there were done some modifications, you'd like to test those.

5) I have some sympathy for `%exclude %{gem_docdir}/rdoc` (because the situation could be better), but is it really worth of it?

Comment 2 Benson Muite 2025-04-06 12:00:56 UTC
(In reply to Vít Ondruch from comment #1)
> A few comments.
> 
> 1) If you keep something like `#Source:
> https://rubygems.org/gems/%%{gem_name}-%%{version}.gem`, you can try to go
> with `%dnl`. I.e. `%dnl Source:
> https://rubygems.org/gems/%{gem_name}-%{version}.gem`. That way, you don't
> have to escape the `%`.
> 

Thanks.

> 2) What is the reason to go with the upstream tarball anyway? Is it the
> test? Isn't it better to include just the test separately?
> 

The gem does not contain tests. It seems more maintenance to package tests separately, would still need to get source from git repository.

> 3) I'd suggest to improve the `description`. I see it comes from the
> .gemspec like that, but we don't need to stick with that. The initial
> paragraph from the README would be likely much better description.
> 

Ok. Done.

> 4) We typically execute the test suite under `.%{gem_instdir}`. And this
> actually might be related to (2). The reason is that the content in
> `.%{gem_instdir}` is what is used in `%install` section. If there were done
> some modifications, you'd like to test those.
>

Tests are not installed, .%{gem_instdir} contains:

.
├── LICENSE
├── README.md
└── lib
    └── tcxread.rb

Can raise an issue upstream if this is non-standard.
 
> 5) I have some sympathy for `%exclude %{gem_docdir}/rdoc` (because the
> situation could be better), but is it really worth of it?

Guess can ask upstream. For python, found that sphinx can generate different themes,
and there is a theme in Fedora without javascript:

https://gitlab.com/lv2/sphinx_lv2_theme

Would be good to make something like this for the Ruby ecosystem or at least the Fedora
Ruby ecosystem. A single page html file need not use javascript since and fonts can
be shared across all gem documentation. Most html viewers have search functionality. There
is also ridoc. It seems that there is interest in RDoc themes:
https://www.rorvswild.com/blog/2024/rorvswild-rdoc-theme
https://github.com/BaseSecrete/rorvswild-theme-rdoc
Though may want to choose a Fedora color palette.

Updated:
spec: https://fed500.fedorapeople.org/rubygem-tcxread.spec
srpm: https://fed500.fedorapeople.org/rubygem-tcxread-0.2.0-1.fc43.src.rpm

Comment 3 Fedora Review Service 2025-04-06 12:06:29 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8864700
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2355279-rubygem-tcxread/fedora-rawhide-x86_64/08864700-rubygem-tcxread/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.


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