Problems with core review

[Doug Ledford]

On Tue, 2007-02-06 at 20:34 +0000, Joe Orton wrote:
> On Tue, Feb 06, 2007 at 11:49:11AM -0800, Christopher Stone wrote:
> > Here are the issues in question:
> > 
> > 1) Replace use of $RPM_SOURCE_DIR with %{SOURCEx}
> > 
> > I asked about this in #fedora-extras since I did not understand
> > rpmlints Error message. f13 responded by saying you should just use
> > %{SOURCEx}.
> > 
> > I agree with f13 on this issue because it is easier to identify in the
> > spec file where the source files are used.
> 
> Con: it makes renumering Sources a pain,

Eh?  RPM is perfectly happy with sparse numbering, your choice to
renumber is just you being anal (not to mention the already mentioned
alternatives that make it easy, such as sed).

>  it's harder to use since you 
> have to remember numbers not filenames.

Generally speaking, not if you use complete install commands where the
target file name is specified.  The target file name should clearly
indicate to you what SOURCEx maps to.

>   This number/filename mapping 
> trick doesn't scale well as anybody who has maintained spec files with 
> more than a handful of patches knows.

Bullshit.  Maintain the kernel rpm for a while.  Style and consistency
isn't always cheap and it's never free, and it's still worth it for the
fact that it makes it easier for *other* people to work on this code and
understand it, which is the point of a distributed source management
system that people can see into like we are creating now.  Are we just
doing this to waste time or to make a difference?

Take a lesson from the linux kernel community: When working in a
distributed fashion, style doesn't need justification to be enforced,
breaches of style need justification to exist.

Fix it.

> Insufficient justification for change.
> 
> > 2) Add empty %build section even though its not required
> > 
> > All php-pear packages include an empty %build section and php-pear
> > should not be an exception.  This was disccussed at length when
> > creating the php-pear spec file template.  Ville has real world
> > examples how this can cause problems.
> 
> What are they, how do they apply to this package?
> 
> > Technical reason for changing:  rpm is unpredictable with no %build,
> > consistency among all pear packages
> 
> It's worked predictably for the history of this package.  Insufficient 
> justification for change.

OK, time for my soapbox for a bit.  The point of this revue process,
speaking as someone who's been at Red Hat since 1998 and therefore has
packages built *WELL* before all the guidelines were in place and is
willing to take care of whatever needs taken care of in those packages,
is to bring the packages into sync with the guidelines.  The onus is not
on the Fedora Review committee to justify the guidelines to every would
be kernel RPM maintainer out there, nor is the onus on the fedora
reviewers to reject packages.  The onus is on the maintainer to get
their package accepted and to fix their poor style so that the whole of
the Fedora project has a consistent style that allows an individual
contributor to work on any RPM in the repo and know enough to be able to
jump in and get to work right away.  Unless you have significant
justification to over ride the review committee suggestions,
justification more significant that "I don't wanna!" and justification
that supersedes the goal of creating a uniform, distributed source
management system that promotes ease of contribution, then I suggest you
do the work.

To Jesse: the Fedora project will get exactly as much out of this review
process as we put into it.  No more, no less.  If you let sloppy,
inconsistent style slide, then this is all just an exercise in finger
pointing with no real effect.  IMO, the Fedora community is under no
obligation to accept crap packages, and has every right to assign their
own maintainer to take care of a package should a Red Hat maintainer
prove to be unwilling to work cooperatively with the community.  Stick
to your guidelines and cut this kind of crap off at the knees before it
becomes pervasive throughout the review process and part of the benefit
of the review is lost to purposeless exceptions.

As for the lack of %build, I can understand not needing it, but having
it there doesn't hurt, it makes it more obvious that %build does nothing
intentionally, and it fixes an admitted bug in rpm debug-info handling.
But more importantly, the proper channel to address a disagreement over
whether or not packages that don't need built should have a %build
section in the spec file is to address the fedora community and try to
get the guideline changed, not to just refuse to put it in there.
Unless of course you believe that we should *all* just ignore the
guidelines we don't like because we are "above the law" so to speak. 

> > 3) License tag should change to just "PHP License 3.0"
> 
> I'm not making changes here until a license tag standard is agreed. The 
> current draft proposal on the wiki drops the version altogether.

If there's no agreed upon standard, then nothing exists to change to.
This is the only exception you've made that holds any water.

-- 
Doug Ledford <dledford at redhat.com>
              GPG KeyID: CFBFF194
              http://people.redhat.com/dledford

Infiniband specific RPMs available at
              http://people.redhat.com/dledford/Infiniband
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
URL: <http://listman.redhat.com/archives/fedora-maintainers/attachments/20070207/a55648ae/attachment.sig>