Bug 193787
Summary: | Review Request: scite - Scintilla based text editor | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | cq92j9y+rlkr0w |
Component: | Package Review | Assignee: | Jason Tibbitts <j> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | panemade |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2006-06-06 05:47:22 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: | |||
Bug Blocks: | 163779 |
Description
cq92j9y+rlkr0w
2006-06-01 16:17:28 UTC
Not a Review but some comments:- rpmlint on source package gives W: scite invalid-license Historical Permission Notice and Disclaimer The license seems to me to be equivalent to the MIT license (http://www.opensource.org/licenses/mit-license.php); that's what I'd use in the License: field. Some other comments: You don't seem to use %{optflags}; the package is compiled with -Os instead of the usual Fedora set (which includes FORTIFY_SOURCE and -g so that proper debuginfo packages can be generated). It seems that scintilla is built separately and then statically linked in. Is it reasonable at all to build in a separate package and then dynamically link it in? (Keep in mind that I know nothing about scintilla. It does seem that most of the packages that use scintilla seem to just include a copy of the source, which may be the best way to handle it.) Finally, I just wanted to make sure you understand that sponsorship is generally granted only after you've demonstrated familiarity with the packaging guidelines; generally you do this by commenting on other packages up for review. I personally am reluctant to sponsor someone after looking at just a single submitted package. Hi Jason, I've updated the spec file and SRPM: Spec URL: http://calle255.org/scite/scite.spec SRPM URL: http://calle255.org/scite/scite-1.69-2.src.rpm (In reply to comment #2) > The license seems to me to be equivalent to the MIT license > (http://www.opensource.org/licenses/mit-license.php); that's what I'd use in the License: field. I've changed the license to MIT. Like you said both licenses are functionally equivalent (it is also what Wikipedia says). Now rpmlint doesn't give any warnings. > You don't seem to use %{optflags}; Fixed. > > It seems that scintilla is built separately and then statically linked in. Is > it reasonable at all to build in a separate package and then dynamically link it in? It can surely be done, but it would require a lot of work and I don't think it's worth it. Upstream isn't very interested on making scintilla a shared-library and their recommendation has always been to static-link software using it. Also, scite and scintilla are released together so I'm pretty sure using different versions of scite/scintilla together will break things. The only distro I know of that made scite/scintilla independent packages (making scite dynamically linked) is PLD and it seems they are not doing it anymore. Nevertheless, if you feel this is a major issue I'll try to fix it ;) > Finally, I just wanted to make sure you understand that sponsorship is generally > granted only after you've demonstrated familiarity with the packaging > guidelines; generally you do this by commenting on other packages up for review. > I personally am reluctant to sponsor someone after looking at just a single > submitted package. I see your point, but I'm very sorry to hear this. Though I haven't made any comments on other RRs I've been following the discussions for a while (fedora-extras-list included). This isn't my first RPM but it's my first package for Extras so I'm doing my best to get it right. If you think I'm not following any of the PackagingGuidelines please let me know. Jorge. (In reply to comment #3) (dynamically linking scintilla) > It can surely be done, but it would require a lot of work and I don't think it's > worth it. I can understand that; I was merely curious as to its feasibility since it seems cleaner. But if it's not intended to work that way then there's not much point. > I see your point, but I'm very sorry to hear this. Though I haven't made any > comments on other RRs I've been following the discussions for a while > (fedora-extras-list included). Well, you can always go ahead and make a review comments. (You can't assign bugs to yourself or change their status but you can make comments.) We're really just trying to establish two things: that you understand the packaging guidelines and you're willing to commit to maintaining your packages in the long term. We can guage the former by looking at review comments and the quality of packages you've submitted, but it's hard to guage the latter so we have to look at depth of community involvement. Your quick response to comments is encouraging, and if you have anything you'd like to point me at that would help to reassure me, I'd be happy to take a look. The package itself is fine, although I'd still have to do a full review. For what it's worth: I'm currently a Fedora Ambassador for Colombia. I know Extras and Ambassadors are separate projects, but I became a Fedora Ambassador precisely because I want to be involved with Fedora and help in anyway possible. Anyway, I'll try yo get more involved in RRs discussions :) I was not aware of your community involvement. I think folks who need sponsors should be encouraged to tell us these kinds of things. I will be happy to sponsor you; go ahead and request cvsextras membership. Note: desktop-file-install logs the following warning: /var/tmp/scite-1.69-2.fc6-root-mockbuild/usr/share/applications/fedora-SciTE.desktop: warning: non-standard key "MultipleArgs" lacks the "X-" prefix I believe this should be removed. It seems to be something that was in use some time ago but no longer. Review: * package meets naming and packaging guidelines. * specfile is properly named, is cleanly written and uses macros consistently. * dist tag is present. * build root is correct. * license field matches the actual license. * license is open source-compatible. License text included in package. * source files match upstream: 3f84986922ccf9c21a1481ba85153be6 scite169.tgz 3f84986922ccf9c21a1481ba85153be6 scite169.tgz-srpm * latest version is being packaged. * BuildRequires are proper. * package builds in mock (development, x86_64). * rpmlint is silent. * final provides and requires are sane: scite = 1.69-2.fc6 - libatk-1.0.so.0()(64bit) libcairo.so.2()(64bit) libgdk-x11-2.0.so.0()(64bit) libgdk_pixbuf-2.0.so.0()(64bit) libglib-2.0.so.0()(64bit) libgmodule-2.0.so.0()(64bit) libgobject-2.0.so.0()(64bit) libgthread-2.0.so.0()(64bit) libgtk-x11-2.0.so.0()(64bit) libpango-1.0.so.0()(64bit) libpangocairo-1.0.so.0()(64bit) * no shared libraries are present. * package is not relocatable. * owns the directories it creates. * doesn't own any directories it shouldn't. * no duplicates in %files. * file permissions are appropriate. * %clean is present. * %check is not present; no test suite upstream. * no scriptlets present. * code, not content. * documentation is small, so no -docs subpackage is necessary. * %docs are not necessary for the proper functioning of the package. * no headers. * no pkgconfig files. * no libtool .la droppings. X GUI app, with desktop file. But desktop-file-install complains as above. Hi, I removed the 'MultipleArgs' key and now desktop-file-install doesn't complain. Updated spec & SRPM: Spec URL: http://calle255.org/scite/scite.spec SRPM URL: http://calle255.org/scite/scite-1.69-3.src.rpm (In reply to comment #3) > (In reply to comment #2) > The license seems to me to be equivalent to the MIT license > (http://www.opensource.org/licenses/mit-license.php); that's what I'd use in > the License: field. That's wrong. I've got some involvement with the scintilla project and I'm almost sure both scintilla and scite have ever used the Python license. http://sourceforge.net/projects/scintilla/ License: Python License (CNRI Python License). http://www.opensource.org/licenses/pythonpl.php I can request an authoritative answer upstream if you wish, as I've been participating on scintilla's mailing list for some years now. SciTE's License.txt file contains a copy of the "Historical Permission Notice and Disclaimer" license instead of the Python license. License.txt (scintilla/SciTE) http://scintilla.cvs.sourceforge.net/scintilla/scite/License.txt?view=markup Historical Permission Notice and Disclaimer http://www.opensource.org/licenses/historical.php According to Wikipedia [1]: "It [SciTE] is licensed under a minimal version of the Historical Permission Notice and Disclaimer". [1] http://en.wikipedia.org/wiki/Scite The new package looks good; the only issue I had is fixed. APPROVED Let me know if you need help checking in. (In reply to comment #9) > SciTE's License.txt file contains a copy of the "Historical Permission Notice > and Disclaimer" license instead of the Python license. Sorry for the delay, but on doubt I asked upstream. http://aspn.activestate.com/ASPN/Mail/Message/scintilla-interest/3153640 |