[Bug 800930] Review Request: redeclipse - Multiplayer FPS game based on Cube2

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


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

Brendan Jones <brendan.jones.it at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Depends on|                            |177841(FE-NEEDSPONSOR),
                   |                            |799778

--- Comment #5 from Brendan Jones <brendan.jones.it at gmail.com> 2012-03-07 16:59:18 EST ---

(In reply to comment #4)
> Hello, and thanks for reviewing! :)

Hi, as this is your first package I can't sponsor you but I can at least help
you get it into shape before someone does. 

> 
> I'm in the process of re-uploading the files, but the srpm will obviously take
> a while...

Don't upload a new SRPM until you've got your sources correct - its too big.
Just list them here for now. 

> Or are you saying the patches should be separated out and not extracted from
> the debian sources? I figured it was the right thing to do since these are
> patches that won't get applied upstream (I have asked), and since I wrote the
> patches for Debian initially, it is the upstream for these patches, no?

Correct, just include  the unedited source tarball along with the required
patches as diffs (%Patch0, %Patch1 .. etc). Part of the review process is to
checksum the upstream tarball against what is in the SRPM. Also, don't include
any of the debian license files. 

You also shouldn't be building any bundled libraries (enet for example), you'll
have to remove those sources in your %prep section and ad a BuildRequires: enet
>= 1.3. I've added a blocker on the bug you've raised for this.

> 
> The icon-fix patch is an exception, that one is applied upstream, the header
> should indicate that, should I be including this a a separate Patch# instead?
> (I was pulling it in since it already exists in the debian sources)

Your patches should be split as you deem appropriate. This is a good candidate
for a separate patch. If upstream fix/apply something in a subsequent release
it is much easier to remove a single %patch, than it is to selectively
edit/recreate a large one.



> I am not using svn snapshots since the SVN version will become (is?)
> incompatible with the released version. Maybe there would be reason to do a
> redeclipse-svn package sometime, but at the moment I do not think there is.

No problem

I'll have a closer look at your spec tomorrow.

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