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!
> 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
> >
> >
>
screenfetch.diff
Description: Binary data
