lucenenet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From synhershko <...@git.apache.org>
Subject [GitHub] lucenenet issue #176: Ported QueryParser
Date Mon, 15 Aug 2016 21:44:22 GMT
Github user synhershko commented on the issue:

    https://github.com/apache/lucenenet/pull/176
  
    Hi, sorry it took me so long to review. I wanted to review properly and give proper feedback
to complement your hard (and very professional!) work here.
    
    So,
    
    1. This makes sense. This is in-line with our .NETification spirit, and as far as we don't
remove important functionality I'd say this makes sense and can be removed altogether - or
changed to work in a way that makes sense.
    
    2. This makes sense as well. We took this approach in several other places in the code
base. I agree the guideline here should be code readability / usability / fluidity.
    
    3. Using an Enum does make sense, and indeed better to wait for tests to stabilize.
    
    4. This might diverge from other places in our codebase. Why is it public API anyway?
I can't really see anyone using it, except from people trying to customize the SimpleQueryParser
- which is simplistic in it's nature. I'd say let's make it nullable and either accept it's
a public API with not much expected usage, or hide it as non-public API. WDYT?
    
    5. From my familiarity with the Java code base, you pass a flag to enable a feature. I
think it's just confusing test names, and then a bug somewhere. @uschindler can you confirm?
I vaguely recall hitting this one before, will try to look into it too later this week.
    
    6. I will help here. Give me a few (more!) days.
    
    Awesome work dude, thanks very much. I'll leave the decision up to you whether to get
this PR merged or still work on it a bit more until it stabilizes. I will look at items 5
and 6 very soon.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message