Bug 508126
Summary: | Review Request: education-bookmarks - Education SIG spin bookmarks | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Mel Chua <mel> |
Component: | Package Review | Assignee: | Rex Dieter <rdieter> |
Status: | CLOSED NOTABUG | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | low | ||
Version: | rawhide | CC: | a.badger, bjorn, fedora-package-review, mel, msuchy, orion, rdieter, sebastian |
Target Milestone: | --- | Flags: | rdieter:
fedora-review+
kevin: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2016-02-08 14:20:16 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | 432265 | ||
Bug Blocks: | 506597 |
Description
Mel Chua
2009-06-25 17:50:01 UTC
I can take a look soonish... 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? 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) 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. 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. 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. Hrm, the original spec/srpm links get "404 Not Found" now, please provide current URLs, and we can proceed with the review. 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. 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. 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. 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 New Package CVS Request ======================= Package Name: education-bookmarks Short Description: Education SIG bookmarks Owners: mchua sdz Branches: F-11 F-12 InitialCC: rdieter (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. #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. 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. cvs done. You should probibly hash out if alternatives will be used or something else before importing however. Anyone want to step in for "alternatives" http://dailypackage.fedorabook.com/index.php?/archives/6-Wednesday-Why-The-Alternatives-System.html 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 :) 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 :) 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. 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. Hmm this package is marked as dead. |