Re: [google-depan] leppoc commented on revision r63.

[Lee Carver]

codesite...@google.com wrote:
> [google-depan] leppoc commented on revision r63.
> Details are at http://code.google.com/p/google-depan/source/detail?r=63
>
> Score: Positive
>
> General Comment:
> LGTM
>
> Line-by-line comments:
>
> File:
> /reviews/leeca01/DepanApp/prod/src/com/google/devtools/depan/eclipse/views/tools/SelectionEditorTool.java
> (r63)
> ===============================================================================
>
>
> Line 203: installNodeSelectorTab("Node Kind Selector", nodeKindEditor);
> -------------------------------------------------------------------------------
>
> indentation
Fixed ..

> File:
> /reviews/leeca01/DepanCore/prod/src/com/google/devtools/depan/filters/NodeKindMatcher.java
> (r63)
> ===============================================================================
>
>
> Line 41: // TODO Auto-generated method stub
> -------------------------------------------------------------------------------
>
> Is this TODO still valid ?
> Maybe a better name can be returned already ?

The relation count and node kind selectors have revealed some
limitations in the PathExpression model. This really needs a rework to
handle "Node Selector Steps", with a different way of naming steps and
defining composition behaviors. Regardless, NodeKindMatcher cannot be
added to PathExpressions in the current implementation, so giving these
a name is moot.

>
> Respond to these comments at
> http://code.google.com/p/google-depan/source/detail?r=63
> --
> You received this message because you starred this review, or because
> your project has directed all notifications to a mailing list that you
> subscribe to.
> You may adjust your review notification preferences at:
> http://code.google.com/hosting/settings