https://bugzilla.redhat.com/show_bug.cgi?id=971244 --- Comment #3 from Jan Kaluža <jkaluza@xxxxxxxxxx> --- Spec URL: http://jkaluza.fedorapeople.org/adevs.spec SRPM URL: http://jkaluza.fedorapeople.org/adevs-2.6-2.fc18.src.rpm > [!]: License field in the package spec file matches the actual license. > Note: Checking patched sources after %prep for licenses. Licenses found: > "LGPL (v2 or later)", "LGPL (v2 or later) (with incorrect FSF address)", > "Unknown or generated". 306 files have unknown license. Detailed output > of licensecheck in /var/tmp/review/971244-adevs/licensecheck.txt > > In the source code there are LGPLv2+, but the SPEC has GPLv2+ Fixed. > [?]: If the source package does not include license text(s) as a separate > file from upstream, the packager SHOULD query upstream to include it. > > You could do it. Asked upstream via email. > I tried only to run the included tests, but they don't compile: > g++ -fopenmp -g -pedantic -Wall -I../include atomic_test.cpp > In file included from ../include/adevs.h:30:0, > from atomic_test.cpp:3: > ../include/adevs_simulator.h: In instantiation of ‘void adevs::Simulator<X, > T>::exec_event(adevs::Atomic<X, T>*, bool, T) [with X = char; T = double]’: > ../include/adevs_simulator.h:338:3: required from ‘void > adevs::Simulator<X, T>::computeNextState(adevs::Bag<adevs::Event<X, T> >&, > T) [with X = char; T = double]’ > atomic_test.cpp:118:32: required from here > ../include/adevs_simulator.h:620:2: error: ‘notify_state_listeners’ was not > declared in this scope, and no declarations were found by argument-dependent > lookup at the point of instantiation [-fpermissive] > ../include/adevs_simulator.h:620:2: note: declarations in dependent base > ‘adevs::AbstractSimulator<char, double>’ are not found by unqualified lookup > ../include/adevs_simulator.h:620:2: note: use ‘this->notify_state_listeners’ > instead > make: *** [atomic] Error 1 Caused by newer g++. Fixed in current srpm, patch sent upstream. > [x]: Patches link to upstream bugs/comments/lists or are otherwise justified. > > Patch is not justified by comment, but it's purpose is clear. I think you > could improve it a bit (i.e. conditionalize the modelica) and get it > upstream. > Also you don't need to comment out the OPTFLAG, just run 'make OPTFLAG=', > the same for java, so you could stay more close to the upstream. Fixed and sent upstream. > [!]: %check is present and all tests pass. > > You could run the tests. Fixed. > More comments: > > CR + LF line ends in some files, e.g.: /src/Makefile and more > > Please package doc and examples. > > Could you run the tests? > > There should be 'formalisms' instead of 'formalism' in the description as > upstream has. > > You could write in the description of the development package that it could > be used for development or something similar. I think it is not good to have > there the same description as the base package has. > > You can add one more sentence to the desription (copy from the upstream > description). > > Why openmp is not used? I think it can bring some performance gain and > should be generally harmless. > > Please do not mix macros and variables, i.e. $RPM_OPT_FLAGS and e.g. > %{buildroot}. This is not against guidelines (IIRC), but you could be > consistent, i.e. use %{optflags} instead of $RPM_OPT_FLAGS. > > Please report the incorrect FSF address upstream. All that should be done. > You could also package java (if you want :). I'm able to compile it, but I'm not able to test if it works. I have no problem with adding the support later when there will be actual need for it (it means when someone who needs adevs-java package (and who can test it) asks me to do it.) > There is various bundled js code in /docs/api/jquery.js under various > licenses that doesn't seem to be compatible with LGPLv2+ (this is currently > not packaged, but could be as the doc). This is mentioned in -docs subpackage license now. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=96bqCT6rjc&a=cc_unsubscribe _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review