This service will be undergoing maintenance at 00:00 UTC, 2017-10-23 It is expected to last about 30 minutes
Bug 492927 - Review Request: xcowsay - xcowsay displays a cute cow and message
Review Request: xcowsay - xcowsay displays a cute cow and message
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-03-30 15:36 EDT by Fabien Georget
Modified: 2009-05-26 03:52 EDT (History)
3 users (show)

See Also:
Fixed In Version: 1.1-1.fc11
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-05-09 12:43:51 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mtasaka: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Fabien Georget 2009-03-30 15:36:38 EDT
Spec URL: http://www.bde.espci.fr/~george27/xcowsay.spec
SRPM URL: http://www.bde.espci.fr/~george27/xcowsay-1.1-0.1.fc10.src.rpm

Description:
xcowsay displays a cute cow and message on your desktop.
Inspired by the original cowsay.

***
This is my first package so I need a sponsor.
Thanks
Comment 1 Ivan Efremov 2009-04-14 04:27:00 EDT
It's only 'pre-review' since I can't be a sponsor.

1) license should be 'GPLv3+' (according to headers in source files)

2) why do you use 'pre-release' Release tag? consider changing 0.1 -> 1
http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Package_Release

3) consider writing more detailed description

4) preserving timestamps: your Makefile must preserve timestamps using 'install -p'
http://fedoraproject.org/wiki/Packaging:Guidelines#Timestamps 

5) your package doesn't own the directory '%{_datadir}/xcowsay/', see
http://fedoraproject.org/wiki/Packaging/UnownedDirectories

6) note about desktop file:
MUST: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section. If you feel that your packaged GUI application does not need a .desktop file, you must put a comment in the spec file with your explanation.

7) consider adding man pages describing other binaries than xcowsay: xcowdream, xcowthink etc.

Good luck!
Comment 2 Fabien Georget 2009-04-19 05:07:26 EDT
Thanks a lot

I modified the spec according to your advice :
Spec URL: http://www.bde.espci.fr/~george27/xcowsay.spec
SRPM URL: http://www.bde.espci.fr/~george27/xcowsay-1.1-1.fc11.src.rpm

For point 6, I only create a .desktop file for xcowfortune since the others need an argument (the message, or the image, to display)
Comment 3 Mamoru TASAKA 2009-04-27 14:02:55 EDT
Some notes:

* SourceURL
  - Please consider to use %{version} macro on %SOURCE0.
    In this way you probably won't have to rewrite Source0
    when version is upgraded:
    https://fedoraproject.org/wiki/Packaging/SourceURL#Using_.25.7Bversion.7D

* BuildRequires
  - "BR: dbus-devel" is redundant because dbus-glib-devel has
    "Requires: dbus-devel".

* Documents
  - "INSTALL" file is usually for people who want to build/compile
    packages by themselves and not needed for rpm users
  - And "ABOUT-NLS" is almost no related to this package (this
    shows general information for translating).

Then:
-------------------------------------------------------------
NOTE: Before being sponsored:

This package will be accepted with another few work. 
But before I accept this package, someone (I am a candidate) 
must sponsor you.

Once you are sponsored, you have the right to review other 
submitters' review requests and approve the packages formally. 
For this reason, the person who want to be sponsored (like you) 
are required to "show that you have an understanding 
of the process and of the packaging guidelines" as is described
on :
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored

Usually there are two ways to show this.
A. submit other review requests with enough quality.
B. Do a "pre-review" of other person's review request
   (at the time you are not sponsored, you cannot do
   a formal review)

When you have submitted a new review request or have pre-reviewed other 
person's review request, please write the bug number on this bug report 
so that I can check your comments or review request.

Fedora package collection review requests which are waiting for someone to
review can be checked on my wiki page:
http://fedoraproject.org/wiki/User:Mtasaka#B._Review_request_tickets
(Check "No one is reviewing")

Review guidelines are described mainly on:
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
http://fedoraproject.org/wiki/Packaging/Guidelines
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
Comment 4 Fabien Georget 2009-04-28 14:57:18 EDT
I have updated the package :

Spec URL: http://www.bde.espci.fr/~george27/xcowsay.spec
SRPM URL: http://www.bde.espci.fr/~george27/xcowsay-1.1-1.fc11.src.rpm

But I have a question about the files section : Are we allowed (encouraged ?) to not include all files that we don't think it is necessary ? 

I will take a look on review Guidelines. Thank you for this advice.
Comment 5 Mamoru TASAKA 2009-04-29 13:13:01 EDT
Now this package itself is okay. So as I wrote in comment 3,
I will wait for your another review request or your pre-review
of other person's review request.

(In reply to comment #4)
> But I have a question about the files section : Are we allowed (encouraged ?)
> to not include all files that we don't think it is necessary ? 

Yes. Removing unneeded files is always recommended (here it doesn't mean
that unneeded files should be removed from the source tarball
itself)
Comment 6 Mamoru TASAKA 2009-04-29 13:39:53 EDT
By the way from next time please change the release number
every time you modify your spec file to aviod confusion.
Comment 7 Fabien Georget 2009-04-30 10:56:19 EDT
(In reply to comment #6)
> By the way from next time please change the release number
> every time you modify your spec file to aviod confusion.  

Of course :/ sorry !

I discover an interesting package in review request, so I start a pre-review : https://bugzilla.redhat.com/show_bug.cgi?id=498246

I think I need more time to read review done by others people to understand some points.
Comment 8 Mamoru TASAKA 2009-04-30 11:49:15 EDT
Okay.
- Your pre-review seems good for initial comments
- This package itself is okay.
 _____________________________________ 
< This package is approved by mtasaka >
 ------------------------------------- 
        \   ^__^
         \  (oo)\_______
            (__)\       )\/\
                ||----w |
                ||     ||

Please follow the procedure written on:
http://fedoraproject.org/wiki/PackageMaintainers/Join
from "Get a Fedora Account".
After you request for sponsorship a mail will be sent to sponsor 
members automatically (which is invisible for you) which notifies 
that you need a sponsor. After that, please also write on
this bug for confirmation that you requested for sponsorship and
your FAS (Fedora Account System) name. Then I will sponsor you.

If you want to import this package into Fedora 9/10/11, you also have
to look at
http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT
(after once you rebuilt this package on koji Fedora rebuilding system).

If you have questions, please ask me.
Comment 9 Fabien Georget 2009-04-30 12:50:16 EDT
(In reply to comment #8)
> Okay.
> - Your pre-review seems good for initial comments
> - This package itself is okay.
>  _____________________________________ 
> < This package is approved by mtasaka >
>  ------------------------------------- 
>         \   ^__^
>          \  (oo)\_______
>             (__)\       )\/\
>                 ||----w |
>                 ||     ||
>

Thanks :) 

> Please follow the procedure written on:
> http://fedoraproject.org/wiki/PackageMaintainers/Join
> from "Get a Fedora Account".
> After you request for sponsorship a mail will be sent to sponsor 
> members automatically (which is invisible for you) which notifies 
> that you need a sponsor. After that, please also write on
> this bug for confirmation that you requested for sponsorship and
> your FAS (Fedora Account System) name. Then I will sponsor you.

I apply to the packager group.

> If you want to import this package into Fedora 9/10/11, you also have
> to look at
> http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT
> (after once you rebuilt this package on koji Fedora rebuilding system).

I will take a look at this tomorrow.

> If you have questions, please ask me.

Thanks again for helping and sponsoring me.
Comment 10 Fabien Georget 2009-05-01 08:27:42 EDT
(In reply to comment #9)

> > Please follow the procedure written on:
> > http://fedoraproject.org/wiki/PackageMaintainers/Join
> > from "Get a Fedora Account".
> > After you request for sponsorship a mail will be sent to sponsor 
> > members automatically (which is invisible for you) which notifies 
> > that you need a sponsor. After that, please also write on
> > this bug for confirmation that you requested for sponsorship and
> > your FAS (Fedora Account System) name. Then I will sponsor you.
> 
> I apply to the packager group.
> 

Sorry, I forgot to give my login : It is manawy.
Comment 11 Mamoru TASAKA 2009-05-01 09:33:28 EDT
(In reply to comment #10)
> Sorry, I forgot to give my login : It is manawy.  

Well, the email used in your account on FAS differs from
what is used on this bugzilla, which must coincide.
Please change either of them, then I will sponsor you.
Comment 12 Fabien Georget 2009-05-01 14:28:26 EDT
(In reply to comment #11)
> (In reply to comment #10)
> > Sorry, I forgot to give my login : It is manawy.  
> 
> Well, the email used in your account on FAS differs from
> what is used on this bugzilla, which must coincide.
> Please change either of them, then I will sponsor you.  

Sorry, I didn't pay attention to that point. (too many e-mails !)

So :
Account name : manawy
Email : fabien.georget*at*gmail.com
Comment 13 Mamoru TASAKA 2009-05-01 14:43:27 EDT
Okay, now I am sponsoring you. Please follow
"Join" wiki again.
Comment 14 Fabien Georget 2009-05-02 02:01:14 EDT
New Package CVS Request
=======================
Package Name: xcowsay
Short Description: xcowsay displays a cute cow and message on your desktop 
Owners: manawy
Branches: F-10 F-11
InitialCC:
Comment 15 Kevin Fenzi 2009-05-03 23:39:45 EDT
cvs done.
Comment 16 Fedora Update System 2009-05-04 14:03:38 EDT
xcowsay-1.1-1.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/xcowsay-1.1-1.fc11
Comment 17 Fedora Update System 2009-05-04 14:41:01 EDT
xcowsay-1.1-1.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/xcowsay-1.1-1.fc10
Comment 18 Fedora Update System 2009-05-06 19:26:35 EDT
xcowsay-1.1-1.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update xcowsay'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-4255
Comment 19 Fedora Update System 2009-05-09 00:09:34 EDT
xcowsay-1.1-1.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update xcowsay'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-4473
Comment 20 Mamoru TASAKA 2009-05-09 12:43:51 EDT
Now closing.
Comment 21 Fedora Update System 2009-05-25 17:21:12 EDT
xcowsay-1.1-1.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 22 Fedora Update System 2009-05-26 03:52:23 EDT
xcowsay-1.1-1.fc11 has been pushed to the Fedora 11 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.