Bug 432265 - Review Request: astronomy-bookmarks - Fedora astronomy bookmarks
Summary: Review Request: astronomy-bookmarks - Fedora astronomy bookmarks
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Lubomir Rintel
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 508126
TreeView+ depends on / blocked
 
Reported: 2008-02-10 18:15 UTC by Marek Mahut
Modified: 2009-09-20 16:21 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-03-07 10:54:07 UTC
lkundrak: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Marek Mahut 2008-02-10 18:15:02 UTC
Spec URL: http://mmahut.fedorapeople.org/reviews/astronomy-bookmarks/astronomy-bookmarks.spec
SRPM URL: http://mmahut.fedorapeople.org/reviews/astronomy-bookmarks/astronomy-bookmarks-1-1.fc8.src.rpm
Description: This package contains the astronomy bookmarks for Fedora.

Comment 1 Lubomir Kundrak 2008-02-10 18:22:30 UTC
Release:        1%{?dist}

Somewhere I heard that dist tag is not needed in cross-release noarch packages
(was it screensaver-frogs review?).

Your bookmark file embeds a couple of images/icons. Are you sure those are
freely redistributable?

Comment 2 Marek Mahut 2008-02-10 18:46:56 UTC
Legal team, are these icons an issue? I know we have these also in
default-bookmarks.

Comment 3 Marek Mahut 2008-02-19 15:03:40 UTC
15:24:41       spot   marek: if you want to keep the icons, you need to get
explicit approval from each trademark holder to 
                      redistribute the icon under the GFDL.
15:25:20       spot   marek: and honestly, I'm not sure that Fedora would permit
that.
15:25:34       spot   (since the Fedora logo is one of the icons)


Comment 4 Marek Mahut 2008-02-19 15:14:15 UTC
Spec file updated, removed icons,

http://mmahut.fedorapeople.org/reviews/astronomy-bookmarks/astronomy-bookmarks-1-2.fc8.src.rpm

Comment 5 Lubomir Kundrak 2008-02-19 15:50:33 UTC
I find no other problems.

APPROVED

Comment 6 Marek Mahut 2008-02-20 13:36:03 UTC
New Package CVS Request
=======================
Package Name: astronomy-bookmarks
Short Description: Fedora astronomy bookmarks
Owners: mmahut
Branches: F-8
InitialCC: astronomy-sig
Cvsextras Commits: yes

Comment 7 Kevin Fenzi 2008-02-20 20:01:09 UTC
Should we really be importing this before firefox is able to handle more than
one bookmarks package? Is there a bug filed on that? 

Comment 8 Marek Mahut 2008-02-20 21:13:45 UTC
(In reply to comment #7)
> Should we really be importing this before firefox is able to handle more than
> one bookmarks package? Is there a bug filed on that? 

There's no bug. Discussion on #fedora-devel has been based on idea that the
bookmark file is included in firefox itself, which is not true. The standard
bookmark file is in separate package default-bookmarks and thus can be excluded
from the spin.

Comment 9 Kevin Fenzi 2008-02-21 21:17:16 UTC
Sure, but then they both provide the same files, so there should be an explicit
'Conflicts' here? 
http://fedoraproject.org/wiki/Packaging/Conflicts

"Keep in mind that implicit conflicts are NEVER acceptable. If your package
conflicts with another package, then you must either resolve the conflict, or
mark it with Conflicts:"

Comment 10 Marek Mahut 2008-02-22 16:08:30 UTC
(In reply to comment #9)
> Sure, but then they both provide the same files, so there should be an explicit
> 'Conflicts' here? 

You're right, completely forgot about it. My spec file and SRPM are updated.
Chris Aillon told me there are no plans to implement multiple-bookmarks-files
capability into Firefox... So I guess this is our only solution even if it's ugly. 



Comment 11 Kevin Fenzi 2008-02-22 18:03:43 UTC
Yeah, pretty non ideal. ;( Oh well, sometimes thats the way it goes... 

cvs done.

Comment 12 Marek Mahut 2008-03-07 10:54:07 UTC
Build. Thank you Lubomir and Kevin for this review.


Note You need to log in before you can comment on or make changes to this bug.