[asterisk-dev] [Code Review] Documentation explaining PLC

Mark Michelson mmichelson at digium.com
Thu Jun 3 14:32:52 CDT 2010



> On 2010-06-03 12:33:41, Leif Madsen wrote:
> >

Thanks for the quick feedback, Leif. I'll get a new diff posted once I have taken care of your suggestions.


> On 2010-06-03 12:33:41, Leif Madsen wrote:
> > /trunk/doc/tex/plc.tex, line 5
> > <https://reviewboard.asterisk.org/r/688/diff/1/?file=10523#file10523line5>
> >
> >     PLC: generic and native.  (I'd use a colon here)

Sounds reasonable.


> On 2010-06-03 12:33:41, Leif Madsen wrote:
> > /trunk/doc/tex/plc.tex, line 33
> > <https://reviewboard.asterisk.org/r/688/diff/1/?file=10523#file10523line33>
> >
> >     Does this:  <->
> >     
> >     Generate correctly in the PDF? Previously I've found I had to wrap them in $'s:
> >     
> >     $<$-$>$

After inspecting the PDF more closely, yes, it requires the money.


> On 2010-06-03 12:33:41, Leif Madsen wrote:
> > /trunk/doc/tex/plc.tex, line 35
> > <https://reviewboard.asterisk.org/r/688/diff/1/?file=10523#file10523line35>
> >
> >     I read this as "S linear" so I think you want to change 'a' to 'an'.

I didn't read it that way at all. I'll likely just change this sentence not to require an indefinite article.


> On 2010-06-03 12:33:41, Leif Madsen wrote:
> > /trunk/doc/tex/plc.tex, line 74
> > <https://reviewboard.asterisk.org/r/688/diff/1/?file=10523#file10523line74>
> >
> >     I was looking through some manuals to see how we can better define options and filenames, and I think these might work. I haven't had a chance to try them yet though:
> >     
> >     \texttt{transcode\_via\_sln}
> >     
> >     \path{asterisk.conf}

I'll try these out and see how the PDF differs. The same applies for the other places you have suggested such markup.


> On 2010-06-03 12:33:41, Leif Madsen wrote:
> > /trunk/doc/tex/plc.tex, line 38
> > <https://reviewboard.asterisk.org/r/688/diff/1/?file=10523#file10523line38>
> >
> >     I'd have used a semi-colon here instead of a comma.

*dons grammar hat* A semicolon seems badly out of place here since "A and B" is not an independent clause but rather an apositive describing the word "channels." *removes grammar hat*

I can always just change the sentence to be "Consider that Asterisk is bridging channels A and B" though.


> On 2010-06-03 12:33:41, Leif Madsen wrote:
> > /trunk/doc/tex/plc.tex, line 121
> > <https://reviewboard.asterisk.org/r/688/diff/1/?file=10523#file10523line121>
> >
> >     I like to say Local channels instead of local channels to show that it is a channel type.

Can and will do (in all cases).


> On 2010-06-03 12:33:41, Leif Madsen wrote:
> > /trunk/doc/tex/plc.tex, line 126
> > <https://reviewboard.asterisk.org/r/688/diff/1/?file=10523#file10523line126>
> >
> >     Don't think you need to capitalize 'dialplan'

Sure thing.


> On 2010-06-03 12:33:41, Leif Madsen wrote:
> > /trunk/doc/tex/plc.tex, line 130
> > <https://reviewboard.asterisk.org/r/688/diff/1/?file=10523#file10523line130>
> >
> >     I think it might be a bit more clear if you did something like:
> >     
> >     [example]
> >     exten => 1,1,Goto(weasels,1)
> >     
> >     exten => 2,1,Dial(Local/weasels at example/nj)
> >     
> >     exten => file,1,Playback(tt-weasels)

Was the word "file" in the final exten line supposed to be "weasels?" Why does adding a Goto make things more clear? IMO, it adds unneeded complexity.


> On 2010-06-03 12:33:41, Leif Madsen wrote:
> > /trunk/doc/tex/plc.tex, line 118
> > <https://reviewboard.asterisk.org/r/688/diff/1/?file=10523#file10523line118>
> >
> >     Extra newline and tab.

BLOOD?! ON *MY* CODE REVIEW?!!! (I'll take care of it)


> On 2010-06-03 12:33:41, Leif Madsen wrote:
> > /trunk/doc/tex/plc.tex, line 6
> > <https://reviewboard.asterisk.org/r/688/diff/1/?file=10523#file10523line6>
> >
> >     Should you define what an SLINEAR stream is? The all-caps also makes it look like an acronym.

I can add a sentence about that, but I was working under the assumption that most people know what it is already. I'll likely replace all instances of "SLINEAR" with "slin" or something like that.


- Mark


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/688/#review2140
-----------------------------------------------------------


On 2010-06-03 11:41:35, Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/688/
> -----------------------------------------------------------
> 
> (Updated 2010-06-03 11:41:35)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> My recent work with PLC has made it clear that there needs to be better user documentation on the matter, so I have thrown together a little file with some more in-depth explanations on PLC's workings and what is required from a user to get PLC operational.
> 
> 
> Diffs
> -----
> 
>   /trunk/doc/tex/asterisk.tex 267096 
>   /trunk/doc/tex/plc.tex PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/688/diff
> 
> 
> Testing
> -------
> 
> `make pdf` generates what I would expect it to.
> 
> 
> Thanks,
> 
> Mark
> 
>




More information about the asterisk-dev mailing list