[Bug 1036130] New package postgresql-plv8 - javascript language extension for postgresql
https://bugzilla.redhat.com/show_bug.cgi?id=1036130
--- Comment #15 from Christopher Meng ---
Look at other review requests, and then look back to this.
I think you should figure out what still needed.
plv8? postgresql-plv8? Bug title? SPEC name?
LDFLAGS not inserted.
make install DESTDIR=%{buildroot} %{?_smp_mflags}, we only need smp flags
during build, I don't think it's needed in %install.
==
You didn't follow
http://fedoraproject.org/wiki/Join_the_package_collection_maintainers, you only
want to get this package into Fedora.
Sorry, there are a lot of things you haven't done yet.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1036130] New package postgresql-plv8 - javascript language extension for postgresql
https://bugzilla.redhat.com/show_bug.cgi?id=1036130 --- Comment #14 from Mikko Tiihonen --- Is there anything I can do to advance this bug further? I have addressed all the comments given so far about the spec file. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1036130] New package postgresql-plv8 - javascript language extension for postgresql
https://bugzilla.redhat.com/show_bug.cgi?id=1036130 --- Comment #13 from Pavel Raiskup --- (In reply to Zoltan Boszormenyi from comment #12) > My 2 cents: postgresql-plparrot and postgresql-plruby are also not official, > they are not part of the postgresql sources and are Fedora packages. Yes, but we plan to rename these packages as I said before. It is not a reason for naming misleadingly another package. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1036130] New package postgresql-plv8 - javascript language extension for postgresql
https://bugzilla.redhat.com/show_bug.cgi?id=1036130 --- Comment #12 from Zoltan Boszormenyi --- My 2 cents: postgresql-plparrot and postgresql-plruby are also not official, they are not part of the postgresql sources and are Fedora packages. "plv8" would be also confusing on the first sight, like, does it have anything to do with the "pl" (SWI Prolog) package? -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1036130] New package postgresql-plv8 - javascript language extension for postgresql
https://bugzilla.redhat.com/show_bug.cgi?id=1036130 --- Comment #11 from Mikko Tiihonen --- Spec URL: https://www.dropbox.com/s/9cezsd32j7ppj5q/plv8.spec SRPM URL: https://www.dropbox.com/s/8q89vbvggh9dz5o/plv8-1.4.1-1.fc20.src.rpm Description: 3rd revision of package comment 9 review: - fixed findings 1,4,5,6,7,8 and 9 - about 3: the 6E-epel failed due to missing pg 9.2 package DEBUG util.py:264: Getting requirements for plv8-1.4.1-1.el6.src DEBUG util.py:264: Error: No Package found for postgresql-devel >= 9.2 - I found slides that say plv8 supports pg >= 8.4 -> updated spec to allow older versions in hope that rhel builds will succeed - about 2: I had initially named the package postgresql-plv8 but Pavel Raiskup said it will be confused with official postgresql packages. If plv8 is not acceptable either then I propose we decide postgresql- to be from official postgresql sources and postgresql-ext- be any extension and document the practice in https://fedoraproject.org/wiki/Packaging:NamingGuidelines Should I keep plv8 or rename it to postgresql-ext-plv8 ? Other changes: - fixed plv8.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/plv8/README - fixed plv8-debuginfo.x86_64: E: debuginfo-without-sources -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1036130] New package postgresql-plv8 - javascript language extension for postgresql
https://bugzilla.redhat.com/show_bug.cgi?id=1036130 --- Comment #10 from Michael Schwendt --- * rpmlint is not only for checking spec files. Run it on the src.rpm *and* all built rpms at once. * "rawhide" and "f21" are the same currently. * A single scratch-build for only "rawhide" would have been enough during review. Please be gentle with available resources, such as the Fedora build system. Imagine a thousand packagers submit six scratch-build jobs for each minor package update. [...] * Please don't attach packages in bugzilla. Request fedorapeople.org web space if you don't have any other public place where to share the rpms. That's step 2.1.11 here: https://fedoraproject.org/wiki/Join_the_package_collection_maintainers Also, if you followed the process, you would add to lines "Spec URL: ..." and "SRPM URL: ...", which can be evaluated by the fedora-review tool. One would run "fedora-review -b 1036130" and have it perform lots of helpful tests. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1036130] New package postgresql-plv8 - javascript language extension for postgresql
https://bugzilla.redhat.com/show_bug.cgi?id=1036130
Zoltan Boszormenyi changed:
What|Removed |Added
CC||zbos...@pr.hu
--- Comment #9 from Zoltan Boszormenyi ---
Informal review:
1. rpmlint gives this:
$ rpmlint -i -v plv8.spec
plv8.spec:29: W: mixed-use-of-spaces-and-tabs (spaces: line 29, tab: line 4)
The specfile mixes use of spaces and tabs for indentation, which is a cosmetic
annoyance. Use either spaces or tabs for indentation, not both.
plv8.spec: I: checking-url http://plv8js.googlecode.com/files/plv8-1.4.1.zip
(timeout 10 seconds)
plv8.spec: W: invalid-url Source0:
http://plv8js.googlecode.com/files/plv8-1.4.1.zip HTTP Error 404: Not Found
The value should be a valid, public HTTP, HTTPS, or FTP URL.
0 packages and 1 specfiles checked; 0 errors, 2 warnings.
I followed the links on Googe Code and the Source0 URL should be
https://plv8js.googlecode.com/files/plv8-1.4.1.zip , instead of http://...
Tabs and spaces should be consistently used.
2. PostgreSQL extra PL packages are called postgresql-plsomething, calling it
"plv8" doesn't say much.
3. Scratch builds were completed for f19, f20, f21, rawhide, failed for
dist-5E-epel and dist-6E-epel.
http://koji.fedoraproject.org/koji/taskinfo?taskID=6308990
http://koji.fedoraproject.org/koji/taskinfo?taskID=6308983
http://koji.fedoraproject.org/koji/taskinfo?taskID=6309024
http://koji.fedoraproject.org/koji/taskinfo?taskID=6309028
http://koji.fedoraproject.org/koji/taskinfo?taskID=6309011
http://koji.fedoraproject.org/koji/taskinfo?taskID=6309018
4. Remove BuildRoot tag.
5. Group tags are not needed anymore, remove them from the spec file.
6. "%defattr(-,root,root)" is also not needed, remove it.
7. Remove %clean section.
8. Remove "rm -rf %{buildroot}" in %install
9. I tried "make install DESTDIR=..." manually and there is no *.sql file in
%{datadir}, i.e. in $DESTDIR/usr/share. So the extra "rm -f
%{buildroot}%{_datadir}/*.sql" line is not needed in the %install section after
"make install". Remove it.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1036130] New package postgresql-plv8 - javascript language extension for postgresql
https://bugzilla.redhat.com/show_bug.cgi?id=1036130 --- Comment #8 from Mikko Tiihonen --- Created attachment 837221 --> https://bugzilla.redhat.com/attachment.cgi?id=837221&action=edit plv8 src.rpm -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1036130] New package postgresql-plv8 - javascript language extension for postgresql
https://bugzilla.redhat.com/show_bug.cgi?id=1036130 Mikko Tiihonen changed: What|Removed |Added Attachment #830691|0 |1 is obsolete|| --- Comment #7 from Mikko Tiihonen --- Created attachment 837220 --> https://bugzilla.redhat.com/attachment.cgi?id=837220&action=edit Proposed plv8.spec file against plv8 1.4.1 version Thank you for the review. Changes since last time: - fixed the Summary - removed duplicate %changelog line - enhanced the description to mention also CoffeeScript and LiveScript -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1036130] New package postgresql-plv8 - javascript language extension for postgresql
https://bugzilla.redhat.com/show_bug.cgi?id=1036130 --- Comment #6 from Devrim GÜNDÜZ --- Hi, I can sponsor this package. Here is a quick review: * Deps are defined fine * Builds on my Fedora 19 machine * Please remove 2nd %changelog parameter * Please fix summary -- I don't think it belongs to this package I changed the spec a bit for upstream packaging. Package works, and all extensions are loaded w/o any issues. Pavel, can you please remind me how I can sponsor? Regards, Devrim -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1036130] New package postgresql-plv8 - javascript language extension for postgresql
https://bugzilla.redhat.com/show_bug.cgi?id=1036130 Pavel Raiskup changed: What|Removed |Added Blocks||177841 (FE-NEEDSPONSOR) --- Comment #5 from Pavel Raiskup --- Mikko, you are probably not sponsored that means I can't review your package. So I am adding this bug to FE-NEEDSPONSOR tracker - then some sponsor can pick this bugzilla and go through this review. Please, study the [1] document very carefully (especially the Contributor role). You should also submit the comment here in bugzilla a little bit different way - for template you can look at [2]. [1] http://fedoraproject.org/wiki/Package_Review_Process [2] https://bugzilla.redhat.com/enter_bug.cgi?product=Fedora&format=fedora-review Referenced Bugs: https://bugzilla.redhat.com/show_bug.cgi?id=177841 [Bug 177841] Tracker: Review requests from new Fedora packagers who need a sponsor -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1036130] New package postgresql-plv8 - javascript language extension for postgresql
https://bugzilla.redhat.com/show_bug.cgi?id=1036130 --- Comment #4 from Mikko Tiihonen --- Created attachment 830692 --> https://bugzilla.redhat.com/attachment.cgi?id=830692&action=edit plv8 src.rpm -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1036130] New package postgresql-plv8 - javascript language extension for postgresql
https://bugzilla.redhat.com/show_bug.cgi?id=1036130 Mikko Tiihonen changed: What|Removed |Added Attachment #830677|0 |1 is obsolete|| --- Comment #3 from Mikko Tiihonen --- Created attachment 830691 --> https://bugzilla.redhat.com/attachment.cgi?id=830691&action=edit Proposed plv8.spec file against plv8 1.4.1 version -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1036130] New package postgresql-plv8 - javascript language extension for postgresql
https://bugzilla.redhat.com/show_bug.cgi?id=1036130 --- Comment #2 from Pavel Raiskup --- Mikko, thank you for trying to maintain new package for Fedora, btw.! (In reply to Pavel Raiskup from comment #1) > Reassigning to 'Package Review' component. First of all, please could you > use different name? The postgresql-plv8 seems like it comes from > postgresql.conf. Sorry for the typo: s/postgresql.conf/postgresql.spec/. I would be able to review the package, though I am not sponsor so you'll need somebody else (if you are not already sponsored) for sponsoring. Could you post srpm & spec file according to Package Review Process? -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1036130] New package postgresql-plv8 - javascript language extension for postgresql
https://bugzilla.redhat.com/show_bug.cgi?id=1036130 Pavel Raiskup changed: What|Removed |Added CC||package-review@lists.fedora ||project.org Component|postgresql |Package Review Version|20 |rawhide Assignee|prais...@redhat.com |nob...@fedoraproject.org --- Comment #1 from Pavel Raiskup --- Reassigning to 'Package Review' component. First of all, please could you use different name? The postgresql-plv8 seems like it comes from postgresql.conf. Probably something like pgplv8 would be OK (we plan to rename postgresql-ip4r in future to remove the postgresql-* prefix, and same with other packages). Also, this is place you should probably start: http://fedoraproject.org/wiki/Package_Review_Process -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
