Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=705104

--- Comment #11 from Ankur Sinha <sanjay.an...@gmail.com> 2011-06-14 14:03:25 
EDT ---
(In reply to comment #10)
> Please make new releases when you publish your specs. Don't post release 1
> multiple times -- just bump it and make a new changelog entry too. This makes
> it a lot easier for the reviewer and is useful practice.

Sorry, I forgot to update the changelog in my haste. 

> 
> Another thing you can do, is add "-b .does_this_and_that" to the patch macro.
> This creates backups of the patched files and also acts as a help to know 
> which
> patch is which.

Done

> 
> I'd also harmonize the order of BuildArch, Requires and Summary in your
> sub-packages.

Done

> 
> Please comment why you delete contrib, since it isn't obvious.

Done

> 
> "These files consist of the documentation files for %{name}." -- I'm afraid
> that's bad grammar, plus it should describe a package -- not files.

Corrected

> 
> Something is wrong with the main package's description: I think these should 
> be
> 3 paragraphs, but I can only see leading spaces.
> 
> Also consider this paragraph: "FreeDiams is a multi-platform (MacOS, Linux,
> FreeBSD, Windows), free and open source released under the new BSD license." 
> --
> It is more or less clear that it's open source if it's in Fedora and the
> license contradicts -- but you left a comment on that. Personally, I think the
> description could be shorter and more clear.
> 

Corrected. 

> Do you think "export PATH=$PATH:%{_libdir}/qt4/bin/" is really necessary?

The project documentation page says this is required. No harm in including it,
is there?

It now builds correctly, It also functions. Here's a scratch build one can
test:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3131213

SPEC/SRPM:
http://ankursinha.fedorapeople.org/freediams/freediams.spec
http://ankursinha.fedorapeople.org/freediams/freediams-0.5.4-1.fc15.src.rpm

* Tue Jun 14 2011 Ankur Sinha <ankursinha AT fedoraproject DOT org> - 0.5.4-1
- let rpath be
-
http://fedoraproject.org/wiki/PackagingGuidelines#Rpath_for_Internal_Libraries
- Improved upon description
- Added -b to patch application
- Harmomnized order of tags for sub packages
- Add rationale to removal of contrib directory


Thanks,
Ankur

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
_______________________________________________
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

Reply via email to