Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: xvst - Download tool for video clips
https://bugzilla.redhat.com/show_bug.cgi?id=683974
Summary: Review Request: xvst - Download tool for video clips Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: unspecified Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: christoph.korn@web.de QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora
Spec URL: http://188.138.90.189/fedora/xvst.spec SRPM URL: http://188.138.90.189/fedora/xvst-2.4.1-1.fc14.src.rpm Description: xVideoServiceThief (a.k.a xVST) is a tool for downloading your favorite video clips from a lot of video websites (currently supports 76 websites and increasing!).
To introduce myself: Hello, I am in process to try Fedora as my new main or secondary distribution. I want to contribute to the community with this package. It is my first package and I need a sponsor. I am glad about your feedback.
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=683974
Christoph Korn christoph.korn@web.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |177841(FE-NEEDSPONSOR)
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=683974
Mario Blättermann mariobl@gnome.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |mariobl@gnome.org
--- Comment #1 from Mario Blättermann mariobl@gnome.org 2011-03-12 17:40:05 EST --- rpmlint output:
$ rpmlint xvst* xvst.src: W: spelling-error Summary(de) Downloadprogramm -> Nachladeprogramm, Downloaden, Downloade xvst.src: W: spelling-error %description -l en_US xVideoServiceThief -> serviceableness, serviceability, interservice xvst.src: W: spelling-error %description -l de xVideoServiceThief -> servicefreundlich, Bezahlservice xvst.src: W: invalid-url Source1: xvst-fedora.tar.gz xvst.src: W: invalid-url Source0: xvst-2.4.1.tar.gz 1 packages and 0 specfiles checked; 0 errors, 5 warnings.
Some initial issues:
- Please add a downloadable URL for the main source.
- Is there an upstream bugreport for the patches?
- Use either variables or macros. Replace $RPM_BUILD_ROOT with %{buildroot}
- German description: Bitte benutze die Höflichkeitsform. Es ist allgemein unüblich, den Benutzer zu duzen.
FYI, Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2907311
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=683974
Mario Blättermann mariobl@gnome.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |NEW
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=683974
--- Comment #2 from Christoph Korn christoph.korn@web.de 2011-03-12 19:56:42 EST --- Hi,
first thank you for taking the time to review my package. As expected it was not all perfect for my first try but I remember your points for future packages.
You can find the updated files here: http://188.138.90.189/fedora/xvst-2.4.1-2.fc14.src.rpm http://188.138.90.189/fedora/xvst.spec
I tried to include all necessary information in the spec file and I informed upstream about open problems.
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=683974
--- Comment #3 from Christoph Korn christoph.korn@web.de 2011-03-16 14:59:31 EDT --- I added a patch to not try to update the application on startup (same issue as already mentioned in the comment inside the spec file): http://188.138.90.189/fedora/xvst-2.4.1-3.fc14.src.rpm http://188.138.90.189/fedora/xvst.spec
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=683974
--- Comment #4 from Mario Blättermann mariobl@gnome.org 2011-03-17 14:05:15 EDT --- Koji scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2920870
Again, the main source should be defined as a URL, not as the name of the tarball included in the source rpm only. It's no problem that the tarball unpacks to a folder named different from it. Just define a different folder name with
%setup -n xVST_2_4_1_src
Look here: http://www.rpm.org/max-rpm/s1-rpm-inside-macros.html
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=683974
--- Comment #5 from Christoph Korn christoph.korn@web.de 2011-03-17 14:41:59 EDT --- Hi, thanks again for the review. I changes the source RPM accordingly (actually the -c option was required because the zip archive does not contain a top-level directory).
http://188.138.90.189/fedora/xvst-2.4.1-4.fc14.src.rpm http://188.138.90.189/fedora/xvst.spec
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=683974
--- Comment #6 from Christoph Korn christoph.korn@web.de 2011-03-27 10:57:05 EDT --- * Sun Mar 27 2011 Christoph Korn christoph.korn@getdeb.net - 2.4.1-5 - Include the additional files seperately instead of putting them in one tarball.
http://188.138.90.189/fedora/xvst.spec http://188.138.90.189/fedora/xvst-2.4.1-5.fc14.src.rpm
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=683974
Martin Gieseking martin.gieseking@uos.de changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.gieseking@uos.de
--- Comment #7 from Martin Gieseking martin.gieseking@uos.de 2011-07-20 15:04:21 EDT --- Christoph, your package looks almost fine. However, there are a few issues that need some attention:
- As the updater might pull in new files not yet present in the package from a third-party website, this could be a blocker. I asked on IRC how to handle this. It was recommended to ask FESCo [1] whether xvst can be added to Fedora.
- The archive bundles and uses the third-party library qtsingleapplication (in folder src/qtsingleapplication). This is not allowed in Fedora: http://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries
You must remove the bundled sources in %prep and patch the build system of xvst so that the Fedora packages qtsingleapplication and qtsingleapplication-devel are used instead.
- There are a couple of binary files in the zip file, e.g. tools/MigrationTool/Windows/1_8_2a-to-2_0a/rc/brcc32.exe. Please remove them in %prep. You should also ask upstream to drop them from the package.
http://fedoraproject.org/wiki/PackagingGuidelines#No_inclusion_of_pre-built_...
[1] http://fedoraproject.org/wiki/Fedora_Engineering_Steering_Committee
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=683974
Christoph Korn christoph.korn@web.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |CLOSED Resolution| |DEFERRED Last Closed| |2011-07-28 14:13:50
--- Comment #8 from Christoph Korn christoph.korn@web.de 2011-07-28 14:13:50 EDT --- Currently, I do not use Fedora and cannot work on this bug.
I leave the files on my server so someone else can take them and work with them.
package-review@lists.fedoraproject.org