[asterisk-dev] [Code Review] 2811: New pjsip.conf.sample with 100% more pjsip

[rnewton]

> On Aug. 30, 2013, 3:51 a.m., Paul Belanger wrote:
> > /trunk/configs/pjsip.conf.sample, lines 301-314
> > <https://reviewboard.asterisk.org/r/2811/diff/1/?file=45408#file45408line301>
> >
> >     compared to other sections, this is very hard to read.  I would attempt to realign everything.
> 
> Mark Michelson wrote:
>     The biggest problem I foresee is keeping it up-to-date as new options get added. Since the python script you used is not public, it makes it difficult to add onto. Consider adding the python script into the repotools directory so that we can be sure to keep this file updated as changes are made.
>     
>     Some of the information being provided isn't very useful. For instance, in the highlighted section the callerid_tag is listed with (default: , type: Custom). There are two odd things about this:
>     
>     1) I know the default for this is an empty string, but the formatting looks like the default is missing. Consider placing quotation marks around all defaults so it is more clear that an empty string is the default.
>     2) "type: Custom" tells me nothing useful about this option. I assume you got this from the code that registers the object field with sorcery. It uses a custom handler for handling the option, but that doesn't really translate well to public documentation. Really, the callerid_tag is a string. Trying to extract type information from the source with a python script can't be done, and I think the best solution is to just remove the type information altogether from here. If we want to document types, we'll need to do that in the XML documentation some time in the future.

> compared to other sections, this is very hard to read.  I would attempt to realign everything.

I think we could make it look prettier, but I don't think it would make it easier to read. That section is not meant to be read straight through like the example sections. I'd say most would be looking for a particular option or group of options and then just reading the synopsis for those. If they want to read more about a particular option, they'll have to then look at the CLI config help or check out the wiki.

The options themselves on the left side are clearly identifiable when scanning down through them. I'll see if I can adjust the spacing a little bit to help. I was trying to avoid stretching the file out even longer, which seems to happen if the text blocks get thin and tall.

Re: Mark's comments:

I had planned on putting the script under repotools and documenting the process for updating the .conf file. One of my key goals is definitely to make it *easier* to keep these .conf files up to date.

I agree regarding (1) and (2). My python script pulls the Type straight from the XML documentation. There is an attribute "type" on the configOption tags. I'll likely just have the script omit the type for now. We'll also use the ""'s for the blank default.


- rnewton


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


On Aug. 29, 2013, 11:33 p.m., rnewton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2811/
> -----------------------------------------------------------
> 
> (Updated Aug. 29, 2013, 11:33 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22145
>     https://issues.asterisk.org/jira/browse/ASTERISK-22145
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> pjsip.conf!
> 
> The file consists of two main bodies of text.
> 1. Manually written examples - fulfilling a variety of basic configuration scenarios. A few of which are detailed on the ASTERISK-22145 issue.
> 2. A full config option list - Output from a python script I wrote. It takes an xml config dump from Asterisk and parses the pjsip.conf config options out into the format you see in the file. (I may throw that script up here later after I improve it)
> 
> Before the examples there is a blurb talking about where the official documentation is and a brief security notice.
> 
> Notes on the thought process behind what we have so far:
> 
> * We want this to be easy to maintain
> 
> In writing this I found it's easy to turn it into exhaustive documentation and make it very redundant with our other documentation sources. For maintenance sake I trimmed it all back to be about as simple as possible and tried to aim it towards a "quick reference sheet", rather than "demonstrate every config option in action document".
> 
> The full option list is easy to update thanks to it being pulled from a script and not hand edited. 
> 
> * The sample files should start moving towards "sample" files and not be exhaustive documentation.
> * The sample file is not a tutorial.
> 
> The examples provided in the file are geared towards someone who has already read the basics on the wiki[1] or elsewhere. The examples also helpful to serve as context for when you are looking at specific options through the cli config help or referencing the full option list in the bottom of the file. 
> 
> * The examples in the sample file should be simple for a user to understand and modify
> 
> The examples don't have every option listed under them because it makes it a bit confusing to look at. I've tried to arrange them realistically for basic configurations so that it would be easy for you to copy paste options from the big list at the bottom to the examples at the top.
> 
> * Every example is not explained in detail
> 
> I tried to pick out key points or options used in the examples, rather than just explaining them line by line (since that would lead to a lot of redundancy)
> 
> *** Don't provide feedback here for all the individual option names, synopsis, descriptions etc. That is all pulled from the XML config documentation. If you see errors in those areas please file a JIRA issue ***
> 
> Please provide feedback on the example configurations and text that is not in the big list of options at the bottom of the file. That is any obviously wrong configuration (option that couldn't be used in a section), horrible spelling, grammar or logic and flow problems. Suggestions for additional configuration examples that are crucial or essential would be very helpful, especially if you can write the example. Feedback is also very welcome on the formatting of the options list at the bottom. :)
> 
> Moving around all this text, I'm sure I missed quite a few things! Thanks in advance for the help!
> 
> [1]: https://wiki.asterisk.org/wiki/display/AST/Asterisk+12+Installation+and+Configuration
> 
> 
> Diffs
> -----
> 
>   /trunk/configs/pjsip.conf.sample 397957 
> 
> Diff: https://reviewboard.asterisk.org/r/2811/diff/
> 
> 
> Testing
> -------
> 
> I tested pretty much all the configurations in this file or pulled them from other peoples working configurations, there are probably a few option combinations and configurations that I missed, but added in.
> 
> 
> Thanks,
> 
> rnewton
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130830/bb75926b/attachment-0001.htm>