[Insight-developers] Global Code Reviews

Tue Apr 26 17:39:02 EDT 2011

Luis,

I think in a 1 hour tcon we could do the assignments. Then use SLicer4,
SNAP, WikiExamples to establish priorities.

Or, on a rainy day (or non Golf day), I could stop by Kitware and we could
spend a couple of hours and so an initial assigment (assuming you'll buy
lunch).

Bill

On Tue, Apr 26, 2011 at 5:10 PM, Luis Ibanez <luis.ibanez at kitware.com>wrote:

> Hi Bill,
>
> That's a good idea,
>
> Unfortunately we have not made that assignment
> of Review classes to modules.
>
> I would suggest that we identify first the few classes
> from Review that have proved to be of great use to
> applications (e.g. Slicer, SNAP,....), and move those.
>
> In a second pass we can do a full triage of the Review
> directory.  It may turn to be easier to do this assignments
> as we code review each one of the files.
>
>
>    My 2 cents,
>
>
>        Luis
>
>
>
> --------------------------------------------
> On Tue, Apr 26, 2011 at 4:43 PM, Bill Lorensen <bill.lorensen at gmail.com>
> wrote:
> > Luis,
> >
> > You may have already done this, but I think a good first pass would be to
> > assign every class in review to an itk module. Then, when it comes time
> to
> > move or reject them, we'll know where there should or should have gone.
> >
> > Bill
> >
> > On Tue, Apr 26, 2011 at 3:49 PM, Luis Ibanez <luis.ibanez at kitware.com>
> > wrote:
> >>
> >> 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
> >> >> >>
> >> >> >
> >> >> >
> >> >
> >> >
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20110426/41ca8d4d/attachment.htm>