https://bugzilla.redhat.com/show_bug.cgi?id=1290513 Zbigniew Jędrzejewski-Szmek <zbyszek@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review? --- Comment #20 from Zbigniew Jędrzejewski-Szmek <zbyszek@xxxxxxxxx> --- There is a rule to preserve timestamp of *unmodified* files. Once you run sed on a file, it's doesn't apply anymore. People do this thing with .orig file, but it's just a waste of time IMHO. Another thing that people do is to use %{__sed} and %{__rm}, %{__make}, etc. But %{__sed} will always be sed, etc. It's not like %{__python} which can at some point to /usr/bin/python3. There is a rule to use macros for *directories* (and some specific applications as python), which can change over time. Doing it for standard unix programs just decreases readability. OTOH, let's say that upstream changes those files not to have the line you want to delete. You want to guard against that. So you can just do: sed -i '1{/^#!\//d}' %{BUILD_DIR}/python/gui_server.py \ %{BUILD_DIR}/tests/python/test_versionlower.py \ %{BUILD_DIR}/tests/bash/test-versionlower Safer and simpler ;) Thank you for the appdata file. It needs some love though. Usually you want to use "appstream-util validate" locally (to catch as many errors as possible), and put "appstream-util validate-relax" in the spec file (so the build does not break on minor things when new versions of appstream-util are released). Running appstream-util validate PlayOnLinux.appdata.xml: PlayOnLinux.appdata.xml: FAILED: • tag-invalid : <project_license> is not valid [GPLv3]SPDX ID 'GPLv3' unknown • attribute-invalid : <screenshot> width too small [https://jkonecny.fedorapeople.org/images/Screenshot%20from%202016-01-12%2019-14-40.png] Validation of files failed (BTW, shutter is pretty good for screenshots, and gives nice names automatically). Both seem to be valid complaints. Small width causes the screenshots to be surrounded by gray bars in gnome-software. The main window is resizeable, but the installation window has fixed size here. It seems to be some kind of bug, because I cannot even display most of the contents of the window. Upstream bugs like that do not block the review, but you might want to investigate, and/or notify upstream. (Oh, I looked at the sources now. Not a pleasant view.) Appdata file should go into %{_datadir}/appdata/, not /usr/share/applications/PlayOnLinux.appdata.xml. It also appears in /usr/share/playonlinux/etc/PlayOnLinux.appdata.xml, which seems to be some mistake. In the text of the appdata file, and also in %description, I'm missing some sentence or two which say how this applications helps (I'm *guessing* it has a database of commonly used programs, and can list and download them automatically, and is able to configure wine specifically for each program. The list of programs and configuration settings is periodically updated over the web.) rpmbuild complains: warning: File listed twice: /usr/share/playonlinux/lang/locale/ar/LC_MESSAGES/pol.mo warning: File listed twice: /usr/share/playonlinux/lang/locale/ast/LC_MESSAGES/pol.mo warning: File listed twice: /usr/share/playonlinux/lang/locale/bg/LC_MESSAGES/pol.mo warning: File listed twice: /usr/share/playonlinux/lang/locale/bn/LC_MESSAGES/pol.mo ... warning: File listed twice: /usr/share/playonlinux/lang/locale/zh_TW/LC_MESSAGES/pol.mo This is because lang files are stored in a directory which is also listed in %files. Normally lang files would be stored underneath /usr/share/locale. I'm not aware of any issues which would be caused by keeping lang files in current location, so the simplest solutions seems to add: %exclude %{_datadir}/%{name}/lang/locale/bn/LC_MESSAGES/*.mo to %files to avoid the warning. OK, the program runs, displays stuff, and seems to be generally functional. The installation does not work for me, because it has a small fixed size and the buttons at the bottom are cut off. Maybe this is only under wayland? Under normal X the buttons are also cut off, but just a bit. So we're at least 95% of the way there with the package. As far as sponsorship goes, please do two or three reviews of packages from http://fedoraproject.org/PackageReviewStatus/NEW.html. Running fedora-review is a good first step, but please note that the automatically generated template needs to be filled in in various places, and trimmed in others. Not everything the tools say is always correct. Sometimes they are outdated, sometimes they are plain wrong. It's always best to link to the relevant part of the guidelines. Please pick packages that are in the area you are interested in, so that you can finalize the review after you get the packager bit. If you have any questions or issues, I'd always be happy to help (zbyszek at in waw pl, zbyszek on #fedora-devel). -- 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 https://admin.fedoraproject.org/mailman/listinfo/package-review
