On Mon, Sep 26, 2016 at 03:00:21PM -0300, Henrique N. Lengler wrote:
> On Sun, Sep 25, 2016 at 09:19:33AM +0100, Stuart Henderson wrote:
> > From reading, I haven't tried building it:
> > 
> > The following line should not have been changed, It should stay as 512x512:
> > 
> > -share/crawl/dat/tiles/stone_soup_icon-512x512.png
> > +share/crawl/dat/tiles/stone_soup_icon-${MODLUA_DEP_VERSION}2x${MODLUA_DEP_VERSION}2.png
> > 
> > SQLITE_INCLUDE_DIR should use ${LOCALBASE} instead of /usr/local.
> > 
> > REVISION should be removed.
> > 
> > Your ports tree is out of date, we need diffs to be against -current.
> 
> Did the changes and updated the diff to the last source.
> 
> New patch attached.

Next round of feedback:

Your diff breaks the .desktop file which used to work before.
No icon is shown in the menu anymore, and the application won't start
since the desktop file tries to run "/usr/games/crawl-tiles".

There are some minor whitespace issues in your Makefile where you
used spaces instead of tabs, e.g. here:

> +CXXFLAGS +=    "-I${LOCALBASE}/include -I${LOCALBASE}/include/SDL2"
> +MAKE_FLAGS =   CC="${CC}" CXX="${CXX}" HOSTCXX="${CXX}" \

> +MODLUA_VERSION =    5.1

> +WANTLIB +=     GL GLU SDL2 SDL2_image freetype png pthread z

These should be "VAR = \tVAL ... \n", e.g.:
MAKE_FLAGS =    CC="${CC}" CXX="${CXX}" HOSTCXX="${CXX}" \
Sometimes using more than one tab might provide better alignment.

It looks like we could switch the MASTER_SITE URL to HTTPS,
do you agree?

Thanks.

Reply via email to