[Insight-developers] Global Code Reviews

Tue Apr 26 15:49:13 EDT 2011

Sorry,
My bad,
I should have described the process better.

    Luis

-----------------------------------
On Tue, Apr 26, 2011 at 3:10 PM, Bill Lorensen <bill.lorensen at gmail.com> wrote:
> I agree 100%. I thought the original message implied B) then A). A) then B)
> sounds good.
>
> Bill
>
> On Tue, Apr 26, 2011 at 2:43 PM, Luis Ibanez <luis.ibanez at kitware.com>
> wrote:
>>
>> Hi Bill,
>>
>> Let's rephrase it as:
>>
>>
>> A)   "Move the Useful classes out of Review and into final Modules"
>>
>> B)  "Move the remaining Review directory to an External module"
>>
>>
>> Along the lines of (A), we will be reviewing, cleaning and bringing
>> up to ITK standards the following class:
>>
>>                 itkMemoryProbesCollectorBase
>>
>> This will remove the only dependency that the Examples have
>> on the Review directory.
>>
>>
>> The  subsequent question is:
>>
>>   What classes is Slicer needing from the ITK Review directory ?
>>
>> We will list those classes, code review them, bring them up to
>> ITK standards and put them in their final Module locations.
>>
>>
>> ---
>>
>> Review currently has 358 files.
>>
>> About the same that used to be in the old "BasicFilters"
>> directory.
>>
>> Over time we have been confusing:
>>
>> "My app depends on class A that happens to be in Review"
>>
>> with
>>
>> "My app depends on the Review directory...."
>>
>> (where the second englobes 358 files...)
>>
>>
>> Triaging the Review directory is one of the top priorities
>> in the general code review of the toolkit.
>>
>>
>>
>>      Luis
>>
>>
>> -----------------------
>> On Tue, Apr 26, 2011 at 1:45 PM, Bill Lorensen <bill.lorensen at gmail.com>
>> wrote:
>> > I did not mean "clearing" literally.
>> >
>> > My concern is with the external project movement. I think we should
>> > review
>> > the "Review" classes, move/repair the ones that satisfy "some criteria".
>> > Once we have done this, then we can create an External Package.
>> >
>> > There are so many changes to our software process, I'm concerned that
>> > adding
>> > another step (working with an external process) will hurt our
>> > productivity.
>> > I ajm also concerned that there are only a handfull of people that
>> > understand the current, new process.
>> >
>> > Bill
>> >
>> > On Tue, Apr 26, 2011 at 1:40 PM, Bradley Lowekamp
>> > <blowekamp at mail.nih.gov>
>> > wrote:
>> >>
>> >> Hello Bill,
>> >> I am not sure that "clearing" out review it the correct way to view
>> >> thing.
>> >> This phrase implies to me moving everything out of review and into ITK
>> >> proper.
>> >> There are many good filters that people have been using for quite a
>> >> while.
>> >> These certainly should be moved. While there are others that have not
>> >> used
>> >> and still need work. While others should replace filters in ITK or are
>> >> otherwise redundant.
>> >> Luis has certainly put together a good list of things to check, and
>> >> it's a
>> >> good list regarding the effort.
>> >> Please only move filters that you can expect the community to be able
>> >> to
>> >> trust the results of. (itkFFTComplexToComplexImageFilter has no
>> >> implementation, for non-FFTW and
>> >> segfaults, http://www.itk.org/Bug/view.php?id=11934)
>> >> Or perhaps we could start look into the filters where are currently in
>> >> ITK
>> >> and not up to the standards. (BinaryPrunningImageFilter,
>> >> BinaryThinningImageFilter)
>> >> Or perhaps we could start looking into the redundant filters in ITK.
>> >> (ErodeObjectMorphologyImageFilter, and
>> >> DilateObjectMorphologyImageFilter no
>> >> one can tell me how this is different then setting for foreground and
>> >> background values in the BinaryDilateImageFilter )
>> >>
>> >>
>> >> *reads Luis's e-mail*
>> >> On Apr 26, 2011, at 1:12 PM, Luis Ibanez wrote:
>> >>
>> >> As we discussed during the Boston meeting, a global
>> >> code review effort will take place before the Beta release.
>> >>
>> >> The code assignments to contractors that Jim created,
>> >> have been posted at:
>> >>
>> >>
>> >> https://spreadsheets.google.com/a/kitware.com/spreadsheet/ccc?key=t8spmfVf1nHiKKkNnVunkUQ&authkey=CM-C2vcL#gid=0
>> >>
>> >>
>> >> We are organizing the procedure for the code reviews in:
>> >> http://www.itk.org/Wiki/ITK_Release_4/Global_Code_Review
>> >>
>> >>
>> >> One of the first action items will be to move the Review
>> >> directory out of the repository and make it available as
>> >> an External module.
>> >>
>> >>
>> >> Then we will identify the classes from Review that are
>> >> commonly used (e.g. in applications like Slicer), and will
>> >> put those classes in the top priorities of code review to
>> >> move them into their final destination inside of the toolkit.
>> >>
>> >>
>> >>   Please share your thoughts on this process,
>> >>
>> >>
>> >> This process make sense to me.
>> >>
>> >>
>> >>       Thanks
>> >>
>> >>
>> >>            Luis
>> >> _______________________________________________
>> >> Powered by www.kitware.com
>> >>
>> >> Visit other Kitware open-source projects at
>> >> http://www.kitware.com/opensource/opensource.html
>> >>
>> >> Kitware offers ITK Training Courses, for more information visit:
>> >> http://kitware.com/products/protraining.html
>> >>
>> >> Please keep messages on-topic and check the ITK FAQ at:
>> >> http://www.itk.org/Wiki/ITK_FAQ
>> >>
>> >> Follow this link to subscribe/unsubscribe:
>> >> http://www.itk.org/mailman/listinfo/insight-developers
>> >>
>> >> ========================================================
>> >>
>> >> Bradley Lowekamp
>> >>
>> >> Lockheed Martin Contractor for
>> >>
>> >> Office of High Performance Computing and Communications
>> >>
>> >> National Library of Medicine
>> >>
>> >> blowekamp at mail.nih.gov
>> >>
>> >>
>> >> _______________________________________________
>> >> Powered by www.kitware.com
>> >>
>> >> Visit other Kitware open-source projects at
>> >> http://www.kitware.com/opensource/opensource.html
>> >>
>> >> Kitware offers ITK Training Courses, for more information visit:
>> >> http://kitware.com/products/protraining.html
>> >>
>> >> Please keep messages on-topic and check the ITK FAQ at:
>> >> http://www.itk.org/Wiki/ITK_FAQ
>> >>
>> >> Follow this link to subscribe/unsubscribe:
>> >> http://www.itk.org/mailman/listinfo/insight-developers
>> >>
>> >
>> >
>
>