[Insight-developers] Global Code Reviews

Luis Ibanez luis.ibanez at kitware.com
Tue Apr 26 14:43:35 EDT 2011


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
>>
>
>


More information about the Insight-developers mailing list