Re: [PATCH] sysutils/screenfetch

Hi Charlène --

On 08/01/18 16:06, Charlène wrote:
> Hi,
>
> I'm joining a new diff after Brian's comments.
>
> On Wed, 1 Aug 2018 02:17:37 -0400
> Brian Callahan wrote:
>
>> Hi Charlène --
>>
>> On 07/30/18 14:22, Charlène wrote:
>>> Hi,
>>>
>>> I'm proposing several changes to this port:
>>>
>>> Makefile:
>>>
>>> * cleaned extra tabs after '='
>>> * use INSTALL_MAN instead of INSTALL_SCRIPT for the manpage
>>> * added myself as MAINTAINER, currently it's "orphaned"
>>> * incremented REVISION
>>> * added braces for variables
>> Some of these I'm OK with, some I'm not super sure about.
>> Yes, I think it's worthwhile to use INSTALL_MAN.
>> Glad to see you taking MAINTAINER.
>> Glad to see the updated patches.
>>
>> Braces around variables are not strictly needed in this case; it
>> becomes a matter of style in this particular instance. But it might
>> be even better to change PKGNAME to ${DISTNAME:L} and change
>> GH_TAGNAME to v3.8.0 -- that would completely eliminate the need for
>> a V variable in the first place.
> I changed to what you recommend. You made me finally understand
> that bsd.port.mk is a great source of documentation by itself,
> thanks!

Since it looks like we're going this route, Makefile.template says
CATEGORIES comes after PKGNAME.
So the first block should be:
COMMENT
PKGNAME
REVISION

Makefile.template also says that CATEGORIES comes after the GH_*
variables, though I see plenty of ports that put CATEGORIES directly
under DISTNAME/PKGNAME, as it would be if there were no GH_* variables.
According to your diff, CATEGORIES was placed according to
Makefile.template and be kept there.

~Brian

>> I'm more inclined to leave extra tabs in once a port has been
>> imported, unless you're otherwise changing the line anyway, which
>> you're not here. Especially in this case, where things are aligned
>> anyway, I'm not sure removing the extra tabs improves readability.
>> But I guess if you're going to do this, now would be the time.
> I was advised to do so for sysutils/neofetch, so i applied the same
> thing for this port. I entirely agree about the fact that it doesn't
> improve (or worsen) readability here. I've let it as-is for this time,
> but i take note for future works.
>
> Charlène.
>
>> ~Brian
>>
>>> patches, already in upstream's master branch but not in a release:
>>>
>>> * fixed "awk: cannot open /proc/fb" [1]
>>> * fixed "unary operator expected" when no GPU is detected
>>> [2]
>>>
>>> Testing:
>>>
>>> It has been successfully tested using proot, but you'll need to
>>> copy /var/run/dmesg.boot in your chroot to check the output of the
>>> program, as it uses this file for OS detection.
>>>
>>> Comments and OK are welcome!
>>>
>>> Charlène.
>>>
>>> [1]
>>> https://github.com/KittyKatt/screenFetch/commit/dc72b5932e86ba9c4e36110408690abeb2004070
>>> [2]
>>> https://github.com/KittyKatt/screenFetch/commit/8346a75068323692243805fa702d02ec7538f3b9
>>>
>>>