New port needs review: net/erlyvideo

[Ruslan Mahmatkhanov]

16.01.2011 23:44, Jason Helfman пишет:
> On Sun, Jan 16, 2011 at 08:25:18PM +0300, Ruslan Mahmatkhanov thus spake:
>> 16.01.2011 00:31, Ruslan Mahmatkhanov пишет:
>>> Hello!
>>>
>>> I'm finally completed with port of erlyvideo (see erlyvideo.org for
>>> details). But before i'll post PR, i want that somebody review it.
>>> Because i'm sure there is something that can be made better. I'm
>>> attached the diff to make commenting more convenient but you can also
>>> download port directory if you prefer:
>>> http://happy-nation.by.ru/ports/tb/erlyvideo.tgz
>>>
>>> Thanks in advance for any suggestions, critics, corrections.
>>
>> Just in case anybody else noticed it, there was MASTER_SITES fetch error
>> in original tarball. It was pointed out by Kurt Jaeger. Now it was fixed
>> and new tarball reupploaded. There is also updated diff-file:
>> http://happy-nation.by.ru/ports/erlyvideo.diff
>>
>> There is also short usage manual:
>> - install the port
>> - cp /usr/local/etc/erlyvideo/*.conf[ig].sample into just *.conf[ig]
>> - then do `erlyctl start`
>> - place some mpeg file into /var/erlyvideo/movies
>> - go to http://localhost:8082/flowplayer/ with flash-aware web-browser
>> - click play button
>> - you should see your mpeg movie packed to flv and transferred to you
>> via rtmp
>>
>> More complicated usage scenarios (like RTSP H.264 cameras, FLME etc) is
>> available on http://erlyvideo.org/docs.
>>
>> --
>> Regards,
>> Ruslan
>
> Just a couple of notes, so far...
>
> The comment reads redundantly, and is too long so it will not display
> properly when listing with pkg_info:
>
> +COMMENT= Erlyvideo is an RTMP flash streaming server, written in
> erlang
>
> Perhaps consider this:
> RMTP flash streaming server written in erlang

Agree. I will do.

> It appears that git is required for the program:
>
> --- erlyvideo.orig/files/patch-Makefile 1970-01-01 03:00:00.000000000 +0300
> +++ erlyvideo/files/patch-Makefile 2011-01-15 15:03:24.000000000 +0300
> @@ -0,0 +1,21 @@
> +--- Makefile.orig 2010-12-16 00:51:55.000000000 +0300
> ++++ Makefile 2011-01-15 15:03:11.000000000 +0300
> +@@ -19,7 +19,6 @@
> + deps/amf: update_deps
> + + update_deps: rebar.config
> +- [ -d wwwroot/player ] || git clone
> git://github.com/erlyvideo/erlyplayer wwwroot/player
> + ./rebar get-deps
> + +
> However there is no dependency for git, if I am interpretting this
> correctly.
> If this updates, or pulls down code during installation, you may
> wish to package it, or not do this step. I don't believe post-fetch
> fetching
> or updating of files to build a port is allowed through pointyhat. Others
> can speak to that, though.

As you can see this patch indeed eliminates using of git, because this 
part isn't really needed for erlyvideo to work.

> You may wish to consider a for loop on the make install target.
> Are these directories created during the install process of the package? I
> didn't see these directories created outside of the Makefile.

Do you mean something like this?

DIRS=	/var/lib/${PORTNAME}/movies /var/lib/${PORTNAME}/plugins \
	/var/log/${PORTNAME} ${ERLYDIR} ${ETCDIR} ${WWWDIR}
.for dir in ${DIRS}
	${MKDIR} ${dir}
.endfor

>
> I noticed many '.gitignore' files in the pkg-plist. Is this necessary? I've
> never worked with git before, or this application.

No, it isn't needed. I'll try to delete them before install.

> When submitting this as a port through the PR system, I would suggest using
> the shar utility:

Yep, i know. I post the diff just to make it easy to comment in email.

>
> http://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/porting-submitting.html
>
>
> -jgh
>

Thank you for your suggestions!

-- 
Regards,
Ruslan