Bug 501353 - Review Request: ascii - Interactive ASCII name and synonym chart
Summary: Review Request: ascii - Interactive ASCII name and synonym chart
Keywords:
Status: CLOSED CANTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jochen Schmitt
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 510856 (view as bug list)
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-05-18 17:24 UTC by Balaji G
Modified: 2010-01-21 03:25 UTC (History)
7 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-09-16 17:07:49 UTC
Type: ---
Embargoed:
jochen: fedora-review-


Attachments (Terms of Use)

Description Balaji G 2009-05-18 17:24:15 UTC
Spec URL: <http://balajig8.fedorapeople.org/packages/ascii/ascii.spec>
SRPM URL: <hhttp://balajig8.fedorapeople.org/packages/ascii/ascii-3.8-1.fc10.src.rpm>
Description: <The ascii utility provides easy conversion between various byte representations and the American Standard Code for Information Interchange (ASCII) character table. >

Hi 

I just finished packaging Ascii which is my first package and I would appreciate a review so that i could add this into fedora .

Ascii is one of the packages mentioned in the Package Wishlist.

Thanks for your time.

Thanks,
Cheers,
Balaji

Comment 1 Balaji G 2009-05-18 17:30:42 UTC
It would be great if some could sponsor me as this is my first package.

Thanks,
Cheers,
Balaji

Comment 2 Jochen Schmitt 2009-05-18 17:55:34 UTC
OK, I will take a look on it, if the package is ok, I will sponsor you.

Comment 3 Jochen Schmitt 2009-05-18 17:59:00 UTC
At first, it will be nice, if you can upload the source rpm too.

Without the source rpm I can't start to review your packages.

Comment 4 Jochen Schmitt 2009-05-18 18:12:20 UTC
At first some advices

* Please add a full qualified URL from which you can downloaded the upstream tar ball in the Source tag
* GPL is not a valid abbrevious of the GPL license. Please keep in mind, that there are several versions of the GPL. I detailed explaination you may find at
https://fedoraproject.org/wiki/Licensing:Main#Good_Licenses
* Please remove the Packager tag
* You don't need to check for the buildroot before delete it on the beginning of %clean and %install
* Please delete the brackets, if you fill out the predefined fields.

Comment 5 Balaji G 2009-05-19 08:53:59 UTC
Hi Jochen 

Thanks for your replies. I have uploaded the Source RPM and the Spec files at the location http://balajig8.fedorapeople.org/packages/ascii/

Thanks,
Cheers,
Balaji

Comment 6 Balaji G 2009-05-19 16:06:53 UTC
Hi Jochen

I have addressed your review comments and have placed the SPEC file and the SRPM at the following location
http://balajig8.fedorapeople.org/packages/ascii/

Please review the same 

Thanks,
Cheers,
Balaji

Comment 7 Balaji G 2009-05-19 16:07:54 UTC
Hi Jochen

I have addressed your review comments and have placed the SPEC file and the SRPM at the following location for your review.
http://balajig8.fedorapeople.org/packages/ascii/

Thanks,
Cheers,
Balaji

Comment 8 Jochen Schmitt 2009-05-19 16:21:04 UTC
It look like, that you didn't upload a correct release of your package.

Please keep in mind do increase the release number each time if you make a change on your package.

Comment 9 Balaji G 2009-05-19 16:54:17 UTC
Hi Jochen

Sorry for the inconvenience. Yes i had uploaded the wrong file.I have updated the release tag in the Spec and uploaded the SRPM at the location for your review.
http://balajig8.fedorapeople.org/packages/ascii/

Thanks,
Cheers,
Balaji

Comment 10 Jochen Schmitt 2009-05-19 17:26:14 UTC
Good:
+ Basename of the SPEC files matches with packagename
+ Packagename fullfill naming guildelines
+ URL tag shows on proper project homepage
+ Package contains most recent stable release of the application
+ Could download upstream tar ball via spectool -g
+ Package sources matches with upstream
(md5sum: 8fb7540bf2a7a8e1fa0086708ed9b881)
+ Consistently rpm macro usage in this package
+ Package has no subpackages
+ Package has proper license tag
+ License tag exclaim GPLv2 as a valid OSS license
+ Package contains verbatin copy of the license text
+ Package support SMP build
+ Local install and uninstall works fine
+ Call of the application works fine
+ Koji scratch build works fine
+ Package has small %doc stanza, so we need no extra doc subpackage
+ Files has proper file permission
+ All packaged files are owned by the package
+ %files stanza has no duplicated entries
+ No packaged file belong to another package


Bad:
- Copyright headers have only a refernce to the LICENSE/COPYING
  file. Please talk to upstream, that he should include a more
  clean copyright note on the source file
- Please remove the Packager tag
- You don't need to test the existance of the Buildroot before 
  cleaning it.
- build don't honour RPM_OPT_FLAGS
- Rpmlint has warning on source rpm:
pmlint ascii-3.8-2.fc10.src.rpm
ascii.src: W: summary-not-capitalized interactive ASCII name and synonym chart
ascii.src: W: name-repeated-in-summary ASCII
ascii.src: W: non-standard-group Utilities/Text
ascii.src: W: mixed-use-of-spaces-and-tabs (spaces: line 72, tab: line 82)
1 packages and 0 specfiles checked; 0 errors, 4 warnings.
- Rpmlint complaints binary package:
rpmlint ascii-3.8-2.fc10.x86_64.rpm
ascii.x86_64: W: summary-not-capitalized interactive ASCII name and synonym chart
ascii.x86_64: W: name-repeated-in-summary ASCII
ascii.x86_64: W: non-standard-group Utilities/Text
1 packages and 0 specfiles checked; 0 errors, 3 warnings.
- Debuginfo package doesn't contains sources
- Please remove the old Changelog and beginning with a new one.

Comment 11 Rex Dieter 2009-05-20 16:15:21 UTC
imo, preserving historical/old changelog is ok, preferred actually, to provide history and attribution.  (Provided it's not huge and doesn't impact pkg size anyway).

Comment 12 Balaji G 2009-05-20 16:42:12 UTC
Hi Jochen

Thank you so much for your extensive review.I have addressed the review comments and have placed the package and the spec file at the following location for your review.

http://balajig8.fedorapeople.org/packages/ascii/

Thanks,
Cheers,
Balaji

Comment 13 Jochen Schmitt 2009-05-24 19:27:11 UTC
Please wrtie $RPM_BUILD_ROOT or $(RPM_BUILD_ROOT) instead of $"RPM_BUILD_ROOT"

Please create a proper Buildroot defintion https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag

The build step in your package doesn't use the compiler flags defined in $RPM_OPT_FLAGS.

the Debuginfo package doesn't contains the source files of your package. This may be happen because the build step doesn't use the compiler flags defined in $RPM_OPT_FLAGS

Good:
+ Rpmlint is silent on source rpm
+ Rpmlint is silent on binary rpm
+ rpmlint is silent on debuginfo rpm

Comment 14 Jason Tibbitts 2009-07-11 17:13:39 UTC
*** Bug 510856 has been marked as a duplicate of this bug. ***

Comment 15 Jochen Schmitt 2009-07-12 19:25:12 UTC
Ping Balaji

Comment 16 Balaji G 2009-07-14 16:29:10 UTC
Hi Jochen 

Sorry couldn't work on this will start this again and repackage it with your comments :)

Thanks,
Cheers,
Balaji

Comment 17 Balaji G 2009-08-29 15:42:27 UTC
Hi Jochen

First of all sorry for the real delay. I was not in station unexpectedly and hence couldnt do it. I have made the changes and uploaded the SPEC file and the SRC RPM at the following location 

http://balajig8.fedorapeople.org/packages/ascii/

Thanks for your inputs and time. 

Cheers,
Balaji

Comment 18 Jochen Schmitt 2009-09-16 17:07:49 UTC
Unfortunately, I have to recorgnise, that you didn't made your homework.

I have the feeling, that you don't have the base skills to become a fedora packager. 

Because the package you want to provides is on the wishlist, I will close this review request and create an own one.


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