Bug 508126 - Review Request: education-bookmarks - Education SIG spin bookmarks
Summary: Review Request: education-bookmarks - Education SIG spin bookmarks
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Rex Dieter
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 432265
Blocks: EducationTracker
TreeView+ depends on / blocked
 
Reported: 2009-06-25 17:50 UTC by Mel Chua
Modified: 2016-02-08 14:20 UTC (History)
8 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-02-08 14:20:16 UTC
Type: ---
Embargoed:
rdieter: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Mel Chua 2009-06-25 17:50:01 UTC
Spec URL: http://mchua.fedorapeople.org/education-bookmarks/education-bookmarks.spec

SRPM URL: http://mchua.fedorapeople.org/education-bookmarks/education-bookmarks-1-1.fc10.src.rpm

Description: 
This is a package of bookmarks for the Education SIG spin. It contains links to information about the spin and the open source education projects that the spin is designed to be a ready-to-go starter development environment for.

This is my first package; I need a sponsor.

Comment 1 Rex Dieter 2009-06-25 18:03:34 UTC
I can take a look soonish...

Comment 2 Björn Persson 2009-07-19 23:24:43 UTC
I'm not qualified to do a review but here are some informal comments:


RPMlint says:
education-bookmarks.src: W: strange-permission default-bookmarks.html 0600
This doesn't seem to hurt.

education-bookmarks.src: E: invalid-spec-name
The spec file in the source package is called "edu-spin-bookmarks.spec". It should be "education-bookmarks.spec".

education-bookmarks.src:13: W: unversioned-explicit-provides system-bookmarks
This looks perfectly fine to me.

education-bookmarks.src: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 2)
Please replace the tabs with spaces. They serve no purpose and can make the file look ugly in other people's editors.

education-bookmarks.noarch: W: no-documentation
Well, this package doesn't seem to need much documentation.


Source0 should be a complete URL. If this isn't possible, there should be a comment explaining why.


The files section contains:
%dir %{_datadir}/bookmarks
%{_datadir}/bookmarks/default-bookmarks.html

Isn't it easier to write just "%{_datadir}/bookmarks" and get the contents of the directory automatically?

Comment 3 Rex Dieter 2009-09-09 16:18:19 UTC
Just noticed, 
/usr/share/bookmarks/default-bookmarks.html

Conflicts with fedora-bookmarks, which is in the least very strongly discouraged, mind using something else, maybe
/usr/share/bookmarks/education-bookmarks.html ?

(which may or may not work out of the box, would have to check how browsers consume stuff in /usr/share/bookmarks)

Comment 4 Sebastian Dziallas 2009-09-12 11:59:35 UTC
Mel, how do we proceed here? Oh, and could you list this web page instead of the current welcome wiki page? https://fedorahosted.org/education

Rex, I looked at astronomy-bookmarks and it's using the default-bookmarks.html with a conflict, too. Not sure how common this is or if other things would work out, though.

Comment 5 Sebastian Dziallas 2009-09-20 16:01:32 UTC
I just double-checked this. #432265 seems to provide an explanation for the conflicts. Rex, would this be enough to justify a conflict?

Mel, maybe using this URL would already work out: https://fedorahosted.org/education/browser/bookmarks/default-bookmarks.html

Except from the smaller issues noted by Björn (which should be pretty easy to fix), it does look rather good to me. However, I'll need to leave this up to Rex or others, as I'm not a sponsor.

Comment 6 Rex Dieter 2009-09-20 16:21:36 UTC
ok, the precendent has been set, I'm ok with allowing this then (though I'd prefer an explicit
Conflicts: fedora-bookmarks
then.

I'll continue with a formal review now.

Comment 7 Rex Dieter 2009-09-20 16:24:20 UTC
Hrm, the original spec/srpm links get "404 Not Found" now, please provide current URLs, and we can proceed with the review.

Comment 8 Mel Chua 2009-09-21 03:59:51 UTC
My apologies; I restructured my fedorapeople.org space a bit. The files are at http://mchua.fedorapeople.org/packages/education-bookmarks/ now. 

I'm out for training most of this week but should be able to address any comments (including Bjorn's) this weekend.

Comment 9 Rex Dieter 2009-09-21 18:02:49 UTC
OK, 

One last suggestion would be to switch to
License: GFDL
to match what fedora-bookmarks and astronomy-bookmarks use.

But otherwise, this is fairly simple packaging-wise, I'll leave that to you to decide how best to handle the licensing


Please also address the issues raised in comment #2, regarding Source0 URL (either provide full URL, or add a comment explaining why one isn't provided).


Fix those, and I think we're good to go.

Comment 10 Rex Dieter 2009-09-21 18:06:21 UTC
Meh, I won't consider those 2 blockers, but please clarify or address them before building.  thanks.

(apply for access to packager group in FAS, let me know your fas username)

and as your sponsor, please list me as CC: on the cvsadmin request. 


APPROVED.

Comment 11 Mel Chua 2009-09-22 03:51:50 UTC
education-bookmarks.src: E: invalid-spec-name
Fixed: it is now called education-bookmarks.spec

education-bookmarks.src: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab:
line 2)
Fixed: tabs have been replaced with spaces. 

Source0 should be a complete URL.
Fixed: it is.

(I am not sure why "%{_datadir}/bookmarks" is identical to the current files section - it's definitely shorter -  but this seems to work, so I guess it is okay.)

Applied to packager group, FAS username mchua.

Rex has been CC'd on the cvsadmin request.

Thanks, Rex, Bjorn, and Sebastian!

http://mchua.fedorapeople.org/packages/education-bookmarks/education-bookmarks.spec
http://mchua.fedorapeople.org/packages/education-bookmarks/education-bookmarks-1-1.fc10.src.rpm
https://fedorahosted.org/education/browser/bookmarks/default-bookmarks.html

Comment 12 Mel Chua 2009-09-22 03:55:31 UTC
New Package CVS Request
=======================
Package Name: education-bookmarks
Short Description: Education SIG bookmarks
Owners: mchua sdz
Branches: F-11 F-12
InitialCC: rdieter

Comment 13 Chitlesh GOORAH 2009-09-22 09:30:33 UTC
(In reply to comment #3)
> Just noticed, 
> /usr/share/bookmarks/default-bookmarks.html
> 
> Conflicts with fedora-bookmarks, which is in the least very strongly
> discouraged, mind using something else, maybe
> /usr/share/bookmarks/education-bookmarks.html ?
> 
> (which may or may not work out of the box, would have to check how browsers
> consume stuff in /usr/share/bookmarks)  

#001  Source0

Source0:        default-bookmarks.html
should be
Source0:        education-default-bookmarks.html

with
install -p -m 644 %{SOURCE0} $RPM_BUILD_ROOT%{_datadir}/bookmarks/default-bookmarks.html

This prevents upload of another html by mistake.

Comment 14 Chitlesh GOORAH 2009-09-22 09:33:43 UTC
#002  File a bug report against "astronomy-bookmarks" to add

Conflicts:      education-bookmarks

http://cvs.fedoraproject.org/viewvc/rpms/astronomy-bookmarks/devel/astronomy-bookmarks.spec?view=markup

Do not push the education-bookmarks to the repos until the astronomy-bookmarks has been tuned.

Comment 15 Toshio Ernie Kuratomi 2009-09-22 14:41:11 UTC
Regarding the Conflicts, is this something that we should use alternatives for?  We're going to be making more of these packages as the number of spins increases.  As far as I can tell, they're used by the spin creators to make the default bookmark set so they are meant to setup at system creation rather than per user.

Comment 16 Kevin Fenzi 2009-09-22 16:38:14 UTC
cvs done. 

You should probibly hash out if alternatives will be used or something else before importing however.

Comment 17 Chitlesh GOORAH 2009-09-24 15:22:49 UTC
Anyone want to step in for "alternatives"

http://dailypackage.fedorabook.com/index.php?/archives/6-Wednesday-Why-The-Alternatives-System.html

Comment 18 Sebastian Dziallas 2009-10-10 15:06:51 UTC
So how do we proceed here best? File a bug against astronomy-bookmarks to include a "conflicts"? Look into "alternatives" (I've never used them before)?

--Sebastian, who'd love to see this bug closed :)

Comment 19 Chitlesh GOORAH 2009-10-10 19:48:01 UTC
Ask on the mailing list and get the astromony-bookmarks's maintainer involved. We got to find a solution :)

I'm also interested in getting a electronic-lab-bookmarks package :) Better fix it so the everyone can benefit from it :)

Comment 20 Rex Dieter 2010-01-22 18:30:13 UTC
Looks like alternatives is the best/only option then.

I'll see about poking folks (other *-bookmarks owners and such), and see if we can make something happen soon.

Comment 21 Miroslav Suchý 2013-10-22 08:30:40 UTC
Ping
Mel, is something blocking you from import of package into dist-git? This BZ is for 10 months without any update and without any visible blocker.

Comment 22 Miroslav Suchý 2016-02-08 14:20:16 UTC
Hmm this package is marked as dead.


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