Bug 2013614 - Review Request: sugar-view-slides - The View Slides activity is meant to allow the user to read, view the contents of a Zip file
Summary: Review Request: sugar-view-slides - The View Slides activity is meant to allo...
Keywords:
Status: CLOSED COMPLETED
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2021-10-13 11:18 UTC by Ibiam Chihurumnaya
Modified: 2023-10-17 11:57 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2023-10-17 11:57:35 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Ibiam Chihurumnaya 2021-10-13 11:18:45 UTC
Spec URL: https://src.fedoraproject.org/rpms/sugar-view-slides/blob/rawhide/f/sugar-view-slides.spec
SRPM URL: https://chimosky.fedorapeople.org/sugar-view-slides-15-1.fc33.src.rpm
Description: The View Slides activity is meant to allow the XO laptop to read
view the contents of a Zip file containing images named sequentially.
Project Gutenberg has a few books as raw scanned images, and this can
be a useful format for picture books, comic books, magazine articles,
photo essays, etc.

Fedora Account System Username: chimosky

The package was retired in F32 due to FTB but it currently builds but it's been dropped by rawhide.

Comment 1 Robby Callicotte 2021-10-20 23:03:02 UTC
Hello,

I am not a packager yet, so this is an unofficial review.

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed


Issues:
=======
- Package does not use a name that already exists.
  Note: A package with this name already exists. Please check
  https://src.fedoraproject.org/rpms/sugar-view-slides
  See: https://docs.fedoraproject.org/en-US/packaging-
  guidelines/Naming/#_conflicting_package_names


===== MUST items =====

Generic:

[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: Package does not own files or directories owned by other packages.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package must not depend on deprecated() packages.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[x]: Final provides and requires are sane (see attachments).
[!]: Package does not include license text files separate from upstream.
[ ]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
     Note: gpgverify is not used.
[ ]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[ ]: Package should compile and build into binary rpms on all supported
     architectures.
[!]: %check is present and all tests pass.
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: sugar-view-slides-15-1.fc36.noarch.rpm
          sugar-view-slides-15-1.fc36.src.rpm
sugar-view-slides.noarch: E: invalid-lc-messages-dir /usr/share/locale/aym/LC_MESSAGES/org.laptop.ViewSlidesActivity.mo
2 packages and 0 specfiles checked; 1 errors, 0 warnings.




Rpmlint (installed packages)
----------------------------
Cannot parse rpmlint output:


Source checksums
----------------
http://download.sugarlabs.org/sources/honey/ViewSlides/ViewSlides-15.tar.bz2 :
  CHECKSUM(SHA256) this package     : 8654c861a058ae1c81dd23e5022a09007dcda0bb95fb0072d84bd2e2b0b67732
  CHECKSUM(SHA256) upstream package : 8654c861a058ae1c81dd23e5022a09007dcda0bb95fb0072d84bd2e2b0b67732


Requires
--------
sugar-view-slides (rpmlib, GLIBC filtered):
    /usr/bin/python3
    python3-pygame
    sugar



Provides
--------
sugar-view-slides:
    sugar-view-slides



Generated by fedora-review 0.7.6 (b083f91) last change: 2020-11-10
Command line :/usr/bin/fedora-review -n sugar-view-slides
Buildroot used: fedora-rawhide-x86_64
Active plugins: Generic, Shell-api
Disabled plugins: Haskell, fonts, Python, SugarActivity, Java, PHP, Perl, Ocaml, C/C++, R
Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH


--Robby

Comment 2 Ibiam Chihurumnaya 2021-10-20 23:31:54 UTC
Thanks for the review and to address the issue you stated above, this review request isn't for a new package
it's for the package to be added to rawhide as it was dropped because of a FTB so the request is for it to be
added back to the rawhide repository.

Comment 4 Miro Hrončok 2021-10-29 11:33:14 UTC
%build
python3 setup.py build

%install
python3 setup.py install --prefix=%{buildroot}/%{_prefix}



Why not %py3_build and %py3_install? See https://docs.fedoraproject.org/en-US/packaging-guidelines/Python_201x/#_example_python_spec_file




rm %{buildroot}%{_prefix}/share/applications/*.desktop || true


Why do you do this?

Comment 5 Ibiam Chihurumnaya 2021-11-01 09:25:42 UTC
(In reply to Miro Hrončok from comment #4)
> %build
> python3 setup.py build
> 
> %install
> python3 setup.py install --prefix=%{buildroot}/%{_prefix}
> 
> 
> 
> Why not %py3_build and %py3_install? See
> https://docs.fedoraproject.org/en-US/packaging-guidelines/Python_201x/
> #_example_python_spec_file


I used %build and %install to maintain the convention used for the sugar packages, thanks for letting me know about %py3_build and %py3_install.
I'll look at switching them.


> 
> 
> 
> 
> rm %{buildroot}%{_prefix}/share/applications/*.desktop || true
> 
> 
> Why do you do this?

The desktop files create icons for running activities outside of sugar on other desktop environments and running an activity from
it's icon was made to be just from the sugar desktop, that's why they're removed.

Comment 6 Ibiam Chihurumnaya 2021-12-01 13:29:59 UTC
(In reply to Miro Hrončok from comment #4)
> %build
> python3 setup.py build
> 
> %install
> python3 setup.py install --prefix=%{buildroot}/%{_prefix}
> 
> 
> 
> Why not %py3_build and %py3_install? See
> https://docs.fedoraproject.org/en-US/packaging-guidelines/Python_201x/
> #_example_python_spec_file



Tried using both and it doesn't work for sugar activities as the extra arguments passed by %py3_build and %py3_install
are not handled by the sugar-toolkit-gtk3 API so the build fails.

Comment 7 Package Review 2023-09-26 00:45:30 UTC
This is an automatic check from review-stats script.

This review request ticket hasn't been updated for some time. We're sorry
it is taking so long. If you're still interested in packaging this software
into Fedora repositories, please respond to this comment clearing the
NEEDINFO flag.

You may want to update the specfile and the src.rpm to the latest version
available and to propose a review swap on Fedora devel mailing list to increase
chances to have your package reviewed. If this is your first package and you
need a sponsor, you may want to post some informal reviews. Read more at
https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group.

Without any reply, this request will shortly be considered abandoned
and will be closed.
Thank you for your patience.

Comment 8 Ibiam Chihurumnaya 2023-10-13 13:25:19 UTC
@mhroncok Is there anything that's needed from me here? I've been waiting for this to be reviewed.

Comment 9 Miro Hrončok 2023-10-17 09:31:10 UTC
This is assigned to you. Package review requests should be assigned to whoever reviews them or to nobody.

Assigning this to nobody makes sure this is listed as "looking for somebody to do the review".

I've just assigned this to nobody now. The next step is to find somebody to review this. I asked some clarifying questions, but I won't be able to review this anytime soon, sorry. My involvement here was years ago.

Comment 10 Ibiam Chihurumnaya 2023-10-17 11:57:35 UTC
Closing this as the package is no longer orphaned and has been updated.


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