[Insight-developers] Global Code Reviews

Tue Apr 26 15:10:02 EDT 2011

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
> >>
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20110426/9f517d52/attachment-0001.htm>