[Bug 244192] Review Request: eclipse-anyedit - AnyEdit plugin for Eclipse

[bugzilla at redhat.com]

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

Summary: Review Request: eclipse-anyedit - AnyEdit plugin for Eclipse


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





------- Additional Comments From alcapcom at gmail.com  2007-09-13 11:00 EST -------
(In reply to comment #9)
> the patch seems to reflect your packaging preferences rather than Fedora
> packaging requirements.  
> for example, the patch changes a line that is
> cut'n'pasted directly from http://fedoraproject.org/wiki/NativeJava #6.
No, take a look for example on eclipse-changelog and eclipse-mylyn, the features
directories are tagged with %dir macro and the second defattr that you add for
the .so files was not necessary - I just follow the way of doing of other packager.

> we should probably work with Ben Konrath and Andrew Overholt to come up with an
> EclipseAddons packaging guide- that way we can capture the best practices and
> ensure some level of consistency between the eclipse plugin packages.
IMHO without guideline to packaging eclipse bundle the best way is to stay
consistent with the other packages already in.

Perhaps, in a near future we would generate the whole specfile from the
feature.xml using the rpmstubby plugin.

> AFAICT, this requirement is already met.  rpm shows that this directory is
> already owned by the rpm:
> # rpm -qf /usr/share/eclipse/plugins/de.loskutov.anyedit.AnyEditTools_1.8.2
> eclipse-anyedit-1.8.2-1.el5
> am i missing something?
I can be wrong but in all the eclipse packages the features use the %dir tag to
say that the directory is owned by the package. Maybe rpm do that automatically too?

> > - MUST: Permissions on files must be set properly. Executables should be set
> > with executable permissions, for example. Every %files section must include a
> > %defattr(...) line.
> > 
> > NOK. The last parameter of the defattr directive set permissions on directories,
> > something like %defattr(-,root,root,-) seems better.
> > See http://docs.fedoraproject.org/developers-guide/ch-rpm-building.html  for
> > more information about defattr.
> 
> i believe the package currently meets this requirement, because it already
> includes a %defattr line.  however, i will include your suggestion because it
> does seem more explicit.  thank you for bringing defattr's fourth argument to
my attention- i didn't know it existed. :)
:), Me too, before someone ask me to add this arg.

> would you be interested in co-maintaining this package?
Why not ;-)

(In reply to comment #10)
Please, can you attach the feature.xml, so that I can make a try here.


-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.