[Bug 515136] Review Request: gettext-commons - Java internationalization (i18n) library

bugzilla at redhat.com bugzilla at redhat.com
Sat Oct 17 13:01:56 UTC 2009


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=515136





--- Comment #7 from Andrea Musuruane <musuruan at gmail.com>  2009-10-17 09:01:55 EDT ---
(In reply to comment #5)
> Please fix the following:
> 
> I cannot install the javadoc subpackage because of line 35, which reads:
>   Requires:       %{name}-%{version}-%{release}
> It should instead read:
>   Requires:       %{name} = %{version}-%{release}

Fixed.

> You mix $THIS_STYLE and %{this_style} variables, which is bad form so the
> guidelines say. You can easily fix this by replacing all instances of
> $RPM_BUILD_ROOT with %{buildroot}

AFAIK What I did is perfectly acceptable. I didn't use both $RPM_BUILD_ROOT and
%{buildroot} - I only used the first. 

Moreover, all other macros are in the second style because they do not have an
equivalent in the first syntax. 

You can find a great number of spec files that do the same in Fedora CVS.

> This one is your choice, but by convention javadoc is installed into a
> versioned directory %{buildroot}%{_javadocdir}/%{name}-%{version} and then an
> unversioned symlink is made %{buildroot}%{_javadocdir}/%{name} to the versioned
> directory. (Just like you did with the jar file.)

Fixed.

> Other points:
> 
> Have you sent the javadoc.patch upstream? Seems like one they could accept
> easily. If you have sent this upstream, please include a link the bug report in
> a comment.

Submitted upstream.

> Once you've addressed these points, I'll give it another look and then probably
> you're ready to go. Thanks for your submission (and thanks for waiting
> patiently for someone to review it)  :-)  

Thanks _a lot_ for the review :)

-- 
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.




More information about the Fedora-package-review mailing list