[Bug 2129390] New: Review Request: nibbles - A snake game written using C++, SDL2, and Opengl with 3D graphics
https://bugzilla.redhat.com/show_bug.cgi?id=2129390
Bug ID: 2129390 Summary: Review Request: nibbles - A snake game written using C++, SDL2, and Opengl with 3D graphics Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Assignee: nobody@fedoraproject.org Reporter: bkeys@bkeys.org QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://download.copr.fedorainfracloud.org/results/bkeys/nibbles/fedora-37-p... SRPM URL: https://bkeys.org/nibbles-1.0-1.fc36.src.rpm Description: Game I made in 2015 as my first stepping stones into OpenGL game development. It's a simple rendition of the classic arcade game nibbles. Collect the red fruit to increase your score while avoiding green pot obstacles. Fedora Account System Username:bkeys
https://bugzilla.redhat.com/show_bug.cgi?id=2129390
--- Comment #1 from Brigham Keys bkeys@bkeys.org --- I have added this package as a copr repo, and it has successfully built on all recent versions of Fedora and CentOS Stream. Here is the link: https://copr.fedorainfracloud.org/coprs/bkeys/nibbles/build/4813230/
https://bugzilla.redhat.com/show_bug.cgi?id=2129390
Robert Scheck redhat-bugzilla@linuxnetz.de changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |redhat-bugzilla@linuxnetz.d | |e Doc Type|--- |If docs needed, set a value
--- Comment #2 from Robert Scheck redhat-bugzilla@linuxnetz.de --- Thank you for your contribution! This is not a formal review, but some findings when having a very first look to it:
- Summary tag is too long and quoted See: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_summary_and_desc... - Description tag is quoted and shorter than summary See: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_summary_and_desc... - Source0 should use URL to upstream tarball See: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/ - Don't disable %debug_package See: https://docs.fedoraproject.org/en-US/packaging-guidelines/Debuginfo/ - License tag doesn't use SPDX See: https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuideline... - Use %{_bindir} instead of /usr/bin See: https://docs.fedoraproject.org/en-US/packaging-guidelines/RPMMacros/ - COPYING listed in %doc and %license -> remove from %doc See: https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuideline... - Requires tag seems to be unnecessary due to SONAME See: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_explicit_require...
Hint: A possible future formal reviewer will follow these guidelines: https://docs.fedoraproject.org/en-US/packaging-guidelines/ReviewGuidelines/
https://bugzilla.redhat.com/show_bug.cgi?id=2129390
Robert Scheck redhat-bugzilla@linuxnetz.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |177841 (FE-NEEDSPONSOR)
--- Comment #3 from Robert Scheck redhat-bugzilla@linuxnetz.de --- Oh, according FAS, you are not a Fedora packager yet; setting FE-NEEDSPONSOR.
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=177841 [Bug 177841] Tracker: Review requests from new Fedora packagers who need a sponsor
https://bugzilla.redhat.com/show_bug.cgi?id=2129390
--- Comment #4 from Robert Scheck redhat-bugzilla@linuxnetz.de --- Brigham, please also read https://docs.fedoraproject.org/en-US/fesco/Packager_sponsor_policy/#requesti... - thank you :)
https://bugzilla.redhat.com/show_bug.cgi?id=2129390
--- Comment #5 from Brigham Keys bkeys@bkeys.org --- Created attachment 1913862 --> https://bugzilla.redhat.com/attachment.cgi?id=1913862&action=edit This is a new SRPM with all of Robert's suggestedchanges
This new SRPM builds successfully on all relevant platforms on copr https://copr.fedorainfracloud.org/coprs/bkeys/nibbles/build/4870465/
https://bugzilla.redhat.com/show_bug.cgi?id=2129390
--- Comment #6 from Brigham Keys bkeys@bkeys.org --- Created attachment 1913864 --> https://bugzilla.redhat.com/attachment.cgi?id=1913864&action=edit New SRPM that passes rpmlint
Please ignore my previous srpm, it has a long description. This attachment is an SRPM that passes rpmlint entirely.
https://bugzilla.redhat.com/show_bug.cgi?id=2129390
Jakub Kadlčík jkadlcik@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |jkadlcik@redhat.com
--- Comment #7 from Jakub Kadlčík jkadlcik@redhat.com --- Thank you for the package Brigham, and also for making the changes
Created attachment 1913864 [details] New SRPM that passes rpmlint
Typically you should always post the updated package the same way you did in the bug description, i.e.
Spec URL: ... SRPM URL: ...
There are tools like `fedora-review` that can parse the BZ comments, find the newest package, download it, run some checks on it, and make the reviewer's life easier.
BuildRequires: freeglut-devel SDL2-devel cmake gcc gcc-c++
This is fine, and I have no problem with this. Just from a personal experience, I find it better to write each dependency on a new line, like this:
BuildRequires: freeglut-devel BuildRequires: SDL2-devel BuildRequires: cmake BuildRequires: gcc BuildRequires: gcc-c++
This way you can have as many dependencies as you need and you are not limited by line length, and also it is much clearer in git diffs what dependency was added and when.
But if you prefer the single-line format, that's fine as well. I just wanted to make it known, that there is an alternative.
%license COPYING
I am a bit uneasy that there are both LICENSE and COPYING files and they are not the same (although they both appear to be GPLv3). Since you are the upstream developer, I think you can easily fix this within the project itself. Or I would just do
%license COPYING %license LICENSE
to install them both.
Otherwise, the package looks good to me.
---
Since this would be your first Fedora package, you will need to get sponsored into the `packager' group before this package can be accepted.
I recently became a sponsor and I would like to sponsor you. That would make it my responsibility to guide you through the processes that you will do, and the tools that you will need as a package maintainer. I would also be there to answer your packaging-related questions, or to help you find somebody who knows the answers.
But before that, I should make sure that you will be able to fulfill your package maintainer responsibilities https://docs.fedoraproject.org/en-US/fesco/Package_maintainer_responsibiliti...
Please take a look here https://docs.fedoraproject.org/en-US/fesco/Packager_sponsor_policy/#requirem...
I think the most beneficial option for you as a sponsoree would be reviewing 2-3 packages from somebody else. It will help you understand the packaging guidelines better but at the same time, it will help somebody else to get also their packages accepted to Fedora.
If you are interested, please ping me on #fedora-devel IRC, my nick is FrostyX, or send me an email, and we can get started. I will explain what is necessary as we go.
https://bugzilla.redhat.com/show_bug.cgi?id=2129390
Jakub Kadlčík jkadlcik@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED
Product: Fedora Version: rawhide Component: Package Review
Jakub Kadlčík jkadlcik@redhat.com has canceled Package Review package-review@lists.fedoraproject.org's request for Jakub Kadlčík jkadlcik@redhat.com's needinfo: Bug 2129390: Review Request: nibbles - A snake game written using C++, SDL2, and Opengl with 3D graphics https://bugzilla.redhat.com/show_bug.cgi?id=2129390
--- Comment #9 from Jakub Kadlčík jkadlcik@redhat.com ---
If you're not interested in reviewing this ticket anymore
I am interested in reviewing this ticket but there is no response from the contributor.
https://bugzilla.redhat.com/show_bug.cgi?id=2129390
Jakub Kadlčík jkadlcik@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(jkadlcik@redhat.c | |om) |
--- Comment #9 from Jakub Kadlčík jkadlcik@redhat.com ---
If you're not interested in reviewing this ticket anymore
I am interested in reviewing this ticket but there is no response from the contributor.
https://bugzilla.redhat.com/show_bug.cgi?id=2129390
AimGrip aimgripofficial@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |aimgripofficial@gmail.com
--- Comment #12 from AimGrip aimgripofficial@gmail.com --- "This is an automatic check from the review-stats script. The review request ticket hasn’t been updated for some time, but it appears the review is still in progress. If so, please respond to this comment, clear the NEEDINFO flag, and reach out to the submitter to proceed. If you’re no longer interested in reviewing, kindly clear the fedora-review flag and reset the assignee so a new reviewer can continue. Without a reply, this request will soon be reset. Updated via AimGrip." https://aimgrip.com/games/wheon-gaming-from-casual-fun-to-esports-glory/
https://bugzilla.redhat.com/show_bug.cgi?id=2129390
--- Comment #13 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Hello, I do realize that this is possibly an old ticket. I am sorry that it hasn't been reviewed yet. Let me trigger the Fedora Review Service to see if the package builds successfully. Hopefully, a green check mark will attract some reviewer.
If I am resurrecting an old ticket that you are not interested in anymore, my apologies, feel free to close it.
[fedora-review-service-build]
https://bugzilla.redhat.com/show_bug.cgi?id=2129390
--- Comment #14 from Jakub Kadlčík jkadlcik@redhat.com --- The Fedora Review Service got a timeout when downloading the SRPM and failed with a traceback instead of commenting on this ticket. I reported an issue for that: https://github.com/FrostyX/fedora-review-service/issues/68
Anyway, please make sure the SRPM is downloadable.
https://bugzilla.redhat.com/show_bug.cgi?id=2129390
William Corlin williamcorlin@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |williamcorlin@gmail.com
--- Comment #15 from William Corlin williamcorlin@gmail.com --- I agree with your suggestions regarding the packaging. It's great to see Brigham's progress, and I think following the guidelines will really help in getting the package approved at https://zapgames.io/
https://bugzilla.redhat.com/show_bug.cgi?id=2129390
Phil Wyett philip.wyett@kathenas.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review? | |needinfo?(bkeys@bkeys.org) CC| |philip.wyett@kathenas.org Status|NEW |ASSIGNED Assignee|nobody@fedoraproject.org |philip.wyett@kathenas.org
--- Comment #16 from Phil Wyett philip.wyett@kathenas.org --- Hi,
I will take this one.
Do we have a progress report for this review request?
The spec file and srpm are not reachable[1] and we could do with the latest of these be uploaded to a location they can be consistently downloaded from.
[1] bkeys.org not reachable.
Regards
Phil
package-review@lists.fedoraproject.org
-
bugzilla＠redhat.com