|Summary:||[patch] x11-wm/ccsm: add LICENSE and option NLS|
|Product:||Ports & Packages||Reporter:||Samy Mahmoudi <samy.mahmoudi>|
|Component:||Individual Port(s)||Assignee:||freebsd-ports-bugs mailing list <ports-bugs>|
|Severity:||Affects Only Me||CC:||freebsd-ports, ndowens|
Description Samy Mahmoudi 2018-08-26 15:29:04 UTC
Created attachment 196569 [details] Patch file generated with svn diff Hi, • Add LICENSE • Add option NLS • Bump PORTREVISION
Comment 1 Nathan 2018-08-26 21:46:52 UTC
+OPTIONS_DEFINE= NLS is a bit out-of-line of the other options
Comment 2 Samy Mahmoudi 2018-08-27 08:44:46 UTC
Indeed, two paragraphs are indented differently from the rest of the text. This is due to "NLS_CONFIGURE_ENABLE=" being too long to indent "nls" on the first level. I thought indenting the options on a second level would improve readability. We could also have reindented the whole text on this second level. Widescreen monitors are rampant. However, sticking to column limits may still be relevant: • for the audience of users with non-widescreen/standard monitors • in case of side-by-side diff • in other cases of multiple panels per screen What do you suggest for that Makefile ? And in general ?
Comment 3 Tobias Kortkamp 2018-11-11 13:43:39 UTC
The indentation looks fine to me after applying the diff. +NLS_USES= gettext-runtime gettext-tools This is equivalent to NLS_USES= gettext The patch looks incomplete to me. It either doesn't do much or is missing changes to pkg-plist to stop installing the locale files. Please confirm that it builds with with NLS=on and NLS=off in Poudriere.
Comment 4 Tobias Kortkamp 2019-01-21 08:57:37 UTC
Actually partially landed now without the NLS changes.
Comment 5 commit-hook 2019-01-21 08:58:03 UTC
A commit references this bug: Author: tobik Date: Mon Jan 21 08:57:15 UTC 2019 New revision: 490835 URL: https://svnweb.freebsd.org/changeset/ports/490835 Log: x11-wm/ccsm: Add LICENSE - While here add missing USES=gnome - Reset maintainer due to the long timeout PR: 230916 Submitted by: Samy Mahmoudi <email@example.com> Approved by: firstname.lastname@example.org (maintainer timeout, ~5 months) Changes: head/x11-wm/ccsm/Makefile
Comment 6 Samy Mahmoudi 2019-02-13 01:24:49 UTC
(In reply to Tobias Kortkamp from comment #4) Hi Tobias! Thank you for your review and your commit.
Comment 7 Samy Mahmoudi 2019-02-13 02:20:47 UTC
Created attachment 201971 [details] Patch file generated with svn diff (In reply to Tobias Kortkamp from comment #3) Thank you for pointing this out. • While splitting gettext into gettext-runtime and gettext-tools clearly allows more control in general, it does not add much here. However, the FreeBSD Porter's Handbook now describes the more-concise-but-not-explicit gettext as being "deprecated": https://www.freebsd.org/doc/en/books/porters-handbook/uses-gettext.html • As you mentioned, the patch was incomplete. I followed your advice and changed pkg-plist accordingly. I also made additional changes to drop gettext and intltool dependencies when NLS is off.
Comment 8 Samy Mahmoudi 2019-02-13 02:23:47 UTC
Created attachment 201972 [details] Poudriere log with NLS on
Comment 9 Samy Mahmoudi 2019-02-13 02:24:18 UTC
Created attachment 201973 [details] Poudriere log with NLS off
Comment 10 Tobias Kortkamp 2019-02-13 09:33:33 UTC
Comment 11 Tobias Kortkamp 2019-02-23 04:09:02 UTC
Comment on attachment 201971 [details] Patch file generated with svn diff Kudos for making it work, but this is adds 8 new patches. Is it really worth it? Do you plan to upstream the changes (assuming they are active)?
Comment 12 Samy Mahmoudi 2019-02-24 04:04:11 UTC
Created attachment 202311 [details] Patch file generated with svn diff (In reply to Tobias Kortkamp from comment #11) > Kudos for making it work, but this is adds 8 new patches. Is it really worth it? I dropped 7 patches with a post-patch definition and a for loop ;-) More seriously, this NLS option was initially suggested by portlint while I was keeping all the compiz ports up-to-date. Thereafter, it seemed worth to me to offer option NLS for users who globally set NLS to off by adding OPTIONS_UNSET+=NLS to their make.conf, even though I am not one of these (I am a French native speaker). > Do you plan to upstream the changes (assuming they are active)? I do not because compiz is dead (to such a point that sources are currently fetched from the FreeBSD distribution cache). There is, however, a community around compiz-reloaded and I have already submitted changes to the build system to increase portability and eventually port compiz-reloaded to FreeBSD (https://gitlab.com/compiz/compiz-core/merge_requests/121). I think they do not have the time to review my work, so I am seriously thinking to create a new port with the changes done within the FreeBSD port system and to resubmit the changes upstream, independently, in small chunks to ease in review. So my goal is to port compiz-reloaded to FreeBSD. During that transition and before considering any drop, we need to keep the compiz ports in the ports tree, preferably in a clean state.