[asterisk-dev] [Code Review]: SDP Media Attribute Interface + H.263/H.264 Format Attribute Modules

[Joshua Colp]

----- Original Message -----
> On 12-07-02 01:26 PM, Joshua Colp wrote:
> >
> >
> >> On June 27, 2012, 11:19 a.m., Mark Michelson wrote:
> >>> I'm giving a "ship it" due to the testing you said you've done.
> >>> I'd feel better if there were unit tests added to ensure that
> >>> SDP is parsed and generated properly.
> >>
> >> Paul Belanger wrote:
> >>      Agreed, I think also adding unit tests before 'ship it' would
> >>      be acceptable here.
> >
> > Unit tests are a separate issue, but I wanted to get eyes on this
> > before going down that road.
> >
> Why separate?  Trying to understand why we as a project don't focus
> more
> on unit tests first before writing code and merging it.

By having sub-tasks it's easier to see where things are and keep track of progress. Granular to a certain degree is good. From the review point of view I *could* have waited and just put everything (this code and unit tests) all up at once but I didn't as I didn't want it to linger and forget about what I've done. This code also won't get merged before the unit tests are done. Of course as you said I could have done unit tests first and written the code to that, it's just a different development methodology.
 
> Is it a time thing or just too much work getting a box setup for unit
> tests?  Also, do people see value in having our code review tool run
> 'tests' before said patch is merged into svn?

There isn't a time thing or too much work issue for me... I wanted to get eyes on it now while it was fresh in my mind.

-- 
Joshua Colp
Digium, Inc. | Software Developer
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at:  www.digium.com  & www.asterisk.org