Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: svn2cl - Create a ChangeLog from a Subversion log
https://bugzilla.redhat.com/show_bug.cgi?id=745547
Summary: Review Request: svn2cl - Create a ChangeLog from a Subversion log Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: ville.skytta@iki.fi QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, package-review@lists.fedoraproject.org Classification: Fedora Story Points: --- Type: ---
http://scop.fedorapeople.org/packages/svn2cl.spec http://scop.fedorapeople.org/packages/svn2cl-0.13-1.fc14.src.rpm
svn2cl was dropped from upstream subversion as of 1.7.0, this brings it back as a separate package (which it originally was).
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=745547
Ville Skyttä ville.skytta@iki.fi changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |jorton@redhat.com
--- Comment #1 from Ville Skyttä ville.skytta@iki.fi 2011-10-12 12:54:06 EDT --- Perhaps Joe could take a look at this - it should be a quick review? The newlines patch that was in the subversion package is no longer needed with this version.
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=745547
Jussi Lehtola jussi.lehtola@iki.fi changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |jussi.lehtola@iki.fi AssignedTo|nobody@fedoraproject.org |jussi.lehtola@iki.fi Flag| |fedora-review+
--- Comment #2 from Jussi Lehtola jussi.lehtola@iki.fi 2011-10-12 17:59:46 EDT --- rpmlint output: svn2cl.noarch: E: explicit-lib-dependency libxslt svn2cl.noarch: W: obsolete-not-provided subversion-svn2cl 2 packages and 0 specfiles checked; 1 errors, 1 warnings.
I think you should Provides: subversion-svn2cl for now.
MUST: The spec file for the package is legible and macros are used consistently. OK MUST: The package must be named according to the Package Naming Guidelines. OK MUST: The spec file name must match the base package %{name}. OK MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines. OK MUST: The License field in the package spec file must match the actual license. OK
MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. OK $ md5sum svn2cl-0.13.tar.gz ../SOURCES/svn2cl-0.13.tar.gz 39764d18ef342145863d5d4e21610e2d svn2cl-0.13.tar.gz 39764d18ef342145863d5d4e21610e2d ../SOURCES/svn2cl-0.13.tar.gz
MUST: The package MUST successfully compile and build into binary rpms. OK MUST: The spec file MUST handle locales properly. N/A MUST: Optflags are used and time stamps preserved. OK MUST: Packages containing shared library files must call ldconfig. N/A MUST: A package must own all directories that it creates or require the package that owns the directory. OK MUST: Files only listed once in %files listings. OK MUST: Debuginfo package is complete. N/A MUST: Permissions on files must be set properly. OK MUST: Large documentation files must go in a -doc subpackage. N/A
MUST: All relevant items are included in %doc. Items in %doc do not affect runtime of application. ?? - I don't think the style sheet svn2html.css belongs in -doc.
MUST: Header files must be in a -devel package. N/A MUST: Static libraries must be in a -static package. N/A MUST: If a package contains library files with a suffix then library files ending in .so must go in a -devel package. N/A MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned, architecture dependent dependency. N/A MUST: Packages does not contain any .la libtool archives. N/A MUST: Desktop files are installed properly. N/A MUST: No file conflicts with other packages and no general names. OK SHOULD: %{?dist} tag is used in release. OK SHOULD: If the package does not include license text(s) as separate files from upstream, the packager should query upstream to include it. OK SHOULD: The package builds in mock. OK
Please add the provide and remove the stylesheet from -doc (if it indeed isn't necessary) before git import. The package has been
APPROVED
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=745547
--- Comment #3 from Ville Skyttä ville.skytta@iki.fi 2011-10-13 02:12:37 EDT --- Thanks for the review!
Will add the provides, but regarding svn2html.css: it is used by the HTML output (--html option) and referred to in the man page. Installing it somewhere else and hardwiring a path to it in svn2html.xsl would make things work out of the box when the HTML is viewed locally. But then again my opinion is that HTML changelogs are not usually generated for local use, and at that point it's easier to just copy the CSS along with the HTML wherever it is going to be hosted than to do that _and_ modify the path to the CSS in the 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=745547
--- Comment #4 from Jussi Lehtola jussi.lehtola@iki.fi 2011-10-13 02:28:35 EDT --- (In reply to comment #3)
Will add the provides, but regarding svn2html.css: it is used by the HTML output (--html option) and referred to in the man page. Installing it somewhere else and hardwiring a path to it in svn2html.xsl would make things work out of the box when the HTML is viewed locally. But then again my opinion is that HTML changelogs are not usually generated for local use, and at that point it's easier to just copy the CSS along with the HTML wherever it is going to be hosted than to do that _and_ modify the path to the CSS in the HTML.
.. okay. Be sure to mention this in the package release notes.
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=745547
--- Comment #5 from Joe Orton jorton@redhat.com 2011-10-13 04:37:51 EDT --- Thanks a lot for creating the package, Ville. It looks good to me.
Missing %defattr in %files?
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=745547
--- Comment #6 from Jussi Lehtola jussi.lehtola@iki.fi 2011-10-13 06:00:21 EDT --- (In reply to comment #5)
Missing %defattr in %files?
It's only necessary for EPEL, modern versions of RPM supply the default flags automatically (in addition to BuildRoot tag, cleaning of buildroot etc).
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=745547
Ville Skyttä ville.skytta@iki.fi changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #7 from Ville Skyttä ville.skytta@iki.fi 2011-10-13 13:29:07 EDT --- Note to SCM admins: the devel branch is already there and populated, just the F-16 branch is needed.
Joe: do you plan to push subversion 1.7.0 to earlier distro versions than F-17? I see there's a build for F-16 it but apparently it is not yet submitted to bodhi.
Package Change Request ====================== Package Name: svn2cl New Branches: f16 Owners: scop
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=745547
--- Comment #8 from Jon Ciesla limb@jcomserv.net 2011-10-13 13:31:31 EDT --- Git done (by process-git-requests).
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=745547
Jon Ciesla limb@jcomserv.net changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |limb@jcomserv.net
--- Comment #9 from Jon Ciesla limb@jcomserv.net 2011-10-13 13:32:06 EDT --- Don't forget to take ownership of the remaining branches. . .
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=745547
--- Comment #10 from Ville Skyttä ville.skytta@iki.fi 2011-10-13 13:48:40 EDT --- Thanks, Jon. I cannot take ownership of the devel branch in pkgdb though because it is marked deprecated and it seems there's nothing I can do about that, there's no "take ownership" button or the like. Can you? (The EPEL branches can be left deprecated, I don't plan to do anything about them for now.)
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=745547
--- Comment #11 from Jon Ciesla limb@jcomserv.net 2011-10-13 14:03:30 EDT --- I thought I'd unretired it. I hadn't. I have now. It should work for now.
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=745547
Ville Skyttä ville.skytta@iki.fi changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |RAWHIDE Last Closed| |2011-10-15 14:44:57
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=745547
Ville Skyttä ville.skytta@iki.fi changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs+ |fedora-cvs?
--- Comment #12 from Ville Skyttä ville.skytta@iki.fi 2011-10-24 13:17:13 EDT --- Package Change Request ====================== Package Name: svn2cl New Branches: f15 Owners: scop
I hear there's a plan to ship subversion 1.7.x to F-15 as well.
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=745547
--- Comment #13 from Jon Ciesla limb@jcomserv.net 2011-10-24 21:44:07 EDT --- Git done (by process-git-requests).
package-review@lists.fedoraproject.org