https://bugzilla.redhat.com/show_bug.cgi?id=1424890 Zbigniew Jędrzejewski-Szmek <zbyszek@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |zbyszek@xxxxxxxxx Depends On| |1422477 Assignee|nobody@xxxxxxxxxxxxxxxxx |zbyszek@xxxxxxxxx --- Comment #7 from Zbigniew Jędrzejewski-Szmek <zbyszek@xxxxxxxxx> --- I added 1422477 as a dep, mostly to have an easy-to-access link to it. It's almost complete anyway. fedora-review says: - Package uses either %{buildroot} or $RPM_BUILD_ROOT Note: Using both %{buildroot} and $RPM_BUILD_ROOT See: http://fedoraproject.org/wiki/Packaging/Guidelines#macros My advice is to stick to %buildroot (or %{buildroot}). That's the commonly used modern form. In fact, it'd be best to replace the make invocation with %make_install, which expands to the exact same line. Those long lines with multiple BuildRequires should be split into separate lines. It's not required, but in my opinion is makes it much easier to spot mistakes and diffs look *much* better. domterm.src:10: W: macro-in-comment %{commit0} domterm.src:44: W: macro-in-comment %{_bindir} → The second one should be replaced with %%{_bindir} to avoid the warning. The first one can stay, if you want to have a valid link in rpmspec -P output. https://fedoraproject.org/wiki/Packaging:Java#BuildRequires_and_Requires says that you need Requires: javapackages-tools, Requires: java. (Either directly or indirectly, but I don't see it either way.) Two big things which is missing, and which is strongly recommended by the guidelines are a .desktop file [https://fedoraproject.org/wiki/Packaging:Guidelines#Desktop_files] and an .appdata file [https://fedoraproject.org/wiki/Packaging:AppData]. In this case an appdata file will be particularly useful, advertising your software and allowing potential users to see how if differers from the many other terminal emulators. In the %description, please add some meat. Why is this package better, what does it do differently than other terminal emulators, what cool features does it sport. This text shouldn't be long, just one or two paragraphs to pique interest. You can reuse most of the text in the appdata file anyway ;) I built and installed the package, on F25. Seems not to work: - in the terminal: $ ldomterm [9066:9066:0328/230720.355575:ERROR:child_thread_impl.cc(762)] Request for unknown Channel-associated interface: ui::mojom::GpuMain and there's also a big blank window which only says "Waiting for localhost..." in the lower left corner. qtdomterm works OK, but it doesn't seem to support HiDPI very well (see attached screenshot). From what I know, it's a general problem with Qt, so I don't expect you to fix it. Just pointing this out in case you need stuff to improve ;) -- I can sponsor you. My requirements apart from this package will be two or three reviews of other packages. Your package is pretty complex, so you'll soak up a lot of the packaging guidelines just for it, but it's always good to see and critique what other people are doing. Please see http://fedoraproject.org/PackageReviewStatus/NEW.html for a list of interesting candidates. Please set up mock, if you haven't already, and base your reviews on fedora-review output, but note that fedora-review does get stuff wrong occasionally and leaves a lot of boxes to be filled manually. In the reviews make a comment that your review is informal because you are not in the packagers group yet. After you are, you can finish those reviews, if nobody else beats you to it. In case of questions, feel free to query me on IRC (I'm zbyszek in #fedora-devel), or by mail. Referenced Bugs: https://bugzilla.redhat.com/show_bug.cgi?id=1422477 [Bug 1422477] libwebsockets-2.2.0 is available -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx