[GitHub] lucenenet issue #185: Ported Core.Util.Fst tests + Core.Util.Packed.TestPack...

2016-10-02 Thread synhershko
Github user synhershko commented on the issue:

https://github.com/apache/lucenenet/pull/185
  
At some point I looked at WeakDictionary as a replacement but didn't go 
forward with it because of some behavior mismatch. Can't really remember now 
exactly what it was. If you are certain that's the way to go I'm okay with it, 
otherwise let's keep a close eye on the beavior of that code...


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] lucenenet pull request #185: Ported Core.Util.Fst tests + Core.Util.Packed.T...

2016-10-02 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/lucenenet/pull/185


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] lucenenet issue #187: Fixed 76 QueryParser Failing Tests

2016-10-02 Thread synhershko
Github user synhershko commented on the issue:

https://github.com/apache/lucenenet/pull/187
  
Let's take the discussion on TimeZone to the dev@ mailing list - can you 
please start a thread there explaining the issue / dilemma and I will follow up?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] lucenenet pull request #187: Fixed 76 QueryParser Failing Tests

2016-10-02 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/lucenenet/pull/187


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] lucenenet issue #187: Fixed 76 QueryParser Failing Tests

2016-10-02 Thread synhershko
Github user synhershko commented on the issue:

https://github.com/apache/lucenenet/pull/187
  
Great job, merging this now


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] lucenenet issue #186: Ported Misc (mostly), Suggest, and Memory

2016-10-02 Thread synhershko
Github user synhershko commented on the issue:

https://github.com/apache/lucenenet/pull/186
  
All 3 were already ported - can you please elaborate on the work done here?

Also some conflicts due to recent merges...


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] lucenenet issue #189: Fixed 24 Failing Analaysis Tests + Completed CharArray...

2016-10-02 Thread synhershko
Github user synhershko commented on the issue:

https://github.com/apache/lucenenet/pull/189
  
Looks good - I will review properly once merge conflicts are resolved (so 
hopefully the diff will be smaller)


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] lucenenet issue #186: Ported Misc (mostly), Suggest, and Memory

2016-10-02 Thread NightOwl888
Github user NightOwl888 commented on the issue:

https://github.com/apache/lucenenet/pull/186
  
The projects were "ported", but they were not compiling and were not part 
of the solution. The fix for FST was to get the bugs out of it because both 
Misc and Suggest depend on it. Although Suggest only depends on 3 types in 
Misc, I ported nearly all of it.

The work done here was to finish up the unfinished implementation of 
Suggest, and to get its tests to pass (no easy feat, mind you).

Anyway, give me a few minutes to fix the merge conflict.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] lucenenet issue #186: Ported Misc (mostly), Suggest, and Memory

2016-10-02 Thread NightOwl888
Github user NightOwl888 commented on the issue:

https://github.com/apache/lucenenet/pull/186
  
Okay, this one has rebased successfully without any issues.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] lucenenet issue #189: Fixed 24 Failing Analaysis Tests + Completed CharArray...

2016-10-02 Thread NightOwl888
Github user NightOwl888 commented on the issue:

https://github.com/apache/lucenenet/pull/189
  
Yep, definitely a smaller diff.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] lucenenet pull request #186: Ported Misc (mostly), Suggest, and Memory

2016-10-02 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/lucenenet/pull/186


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] lucenenet issue #189: Fixed 24 Failing Analaysis Tests + Completed CharArray...

2016-10-02 Thread synhershko
Github user synhershko commented on the issue:

https://github.com/apache/lucenenet/pull/189
  
Merge conflict again :\

LGTM, so feel free to merge and push - you should have permissions to do 
that by now :)


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] lucenenet issue #189: Fixed 24 Failing Analaysis Tests + Completed CharArray...

2016-10-02 Thread NightOwl888
Github user NightOwl888 commented on the issue:

https://github.com/apache/lucenenet/pull/189
  
I haven't tried pushing to the repo, but it doesn't look like I have 
permissions in the GitHub control panel to merge pull requests.

It should only take a couple of minutes to fix this conflict if you bear 
with me. I needed some commits on multiple branches so I cherry-picked (in 
order) in order to ensure the changes were in multiple places. Rebasing works 
without any conflicts.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] lucenenet issue #189: Fixed 24 Failing Analaysis Tests + Completed CharArray...

2016-10-02 Thread synhershko
Github user synhershko commented on the issue:

https://github.com/apache/lucenenet/pull/189
  
The github interface doesn't reflect on your permissions - please try and 
let us know if it doesn't work.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] lucenenet issue #188: Fixed 64 Failing Facet Tests and Finished Facet Implem...

2016-10-02 Thread csharptest
Github user csharptest commented on the issue:

https://github.com/apache/lucenenet/pull/188
  

I have no problem with this. Feel free to extract it. You can probably 
leave the interfaces behind if you don't need them. 


-- Roger
ro...@csharptest.com


> On Oct 2, 2016, at 1:54 AM, eladmarg  wrote:
> 
> there are few files of the LurchTable, so all we need is @csharptest 
permission, and then we can just copy them to the solution and fork the 
relevant parts.
> 
> I think if we'll contact @csharptest, he won't have any opposition.
> The LurchTable is a tested and working solution already, so i think its 
better to use what's working.
> 
> @synhershko is it possible to email @csharptest ?
> 
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.
> 



---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] lucenenet issue #189: Fixed 24 Failing Analaysis Tests + Completed CharArray...

2016-10-02 Thread NightOwl888
Github user NightOwl888 commented on the issue:

https://github.com/apache/lucenenet/pull/189
  
Nope, I have permissions issues:

```
F:\Projects\lucenenet>git push upstream master
Enter passphrase for key '/c/Users/Shad/.ssh/id_rsa':
ERROR: Sorry, but @apache has blocked access to SSH keys created by some 
third-party applications. Your key was created before GitHub tracked keys 
created by applications, so we need your help.

If you personally created this key, you can approve it at:

  https://github.com/settings/ssh/audit/3046281/policy

Otherwise, please upload a new key:

  https://github.com/settings/ssh

Fingerprint: 87:d3:f4:6f:df:78:25:86:fa:cf:20:1a:bc:50:56:ab

[EPOLICYKEYAGE]

fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

```

So I went to the URL and approved the SSH key and then...

```
F:\Projects\lucenenet>git push upstream master
Enter passphrase for key '/c/Users/Shad/.ssh/id_rsa':
ERROR: Permission to apache/lucenenet.git denied to NightOwl888.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

```

I am pushing to the GitHub repo (mirror). When I tried to push to the 
Apache repo, I got:

```
F:\Projects\lucenenet>git push apache master
fatal: remote error: access denied or repository not exported: 
/lucenenet.git
```


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] lucenenet issue #188: Fixed 64 Failing Facet Tests and Finished Facet Implem...

2016-10-02 Thread synhershko
Github user synhershko commented on the issue:

https://github.com/apache/lucenenet/pull/188
  
> A plain old generic Dictionary works fine, but may use more RAM than the 
designers intended.

I am totally okay with that. We should aim to get something out the door 
asap, and then optimize later.  This shouldn't keep us from going ahead, and 
I'd consider this a 80/20 case.

I say let's first release, then see who uses this. Once we see a lot of 
usage, we may reconsider the implementation. WDYT?

> if you really want to thank me for doing this, please spend a weekend 
porting one of the remaining sections that doesn't have an open pull request.

Indeed @eladmarg :)

On that note, @NightOwl888 I have no idea where you are located, but if I 
happen to be in your neighborhood during my travels do let me know and I'll buy 
you beers.

> in another subject, which of the sub-project not ported yet?

Let's have this discussion in the dev@ mailing list please. The list Shad 
provided may be correct, but our priorities are different - the spatial module 
needs work, and there are still failing tests at core. We can also skip 
Analysis.Kuromoji and Analysis.SmartCNand completely now - they aren't worth 
our efforts now. So, Let's have that discussion in the right place so other 
people could chime in as well.

And thank you @csharptest :) -- @eladmarg @NightOwl888 let's see if we can 
wrap this PR soon and move on to finish up the rest of the core and more 
important stuff.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] lucenenet issue #189: Fixed 24 Failing Analaysis Tests + Completed CharArray...

2016-10-02 Thread synhershko
Github user synhershko commented on the issue:

https://github.com/apache/lucenenet/pull/189
  
Ok, email us at private@ and let's figure this out. Ready to merge now?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] lucenenet pull request #189: Fixed 24 Failing Analaysis Tests + Completed Ch...

2016-10-02 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/lucenenet/pull/189


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] lucenenet issue #188: Fixed 64 Failing Facet Tests and Finished Facet Implem...

2016-10-02 Thread NightOwl888
Github user NightOwl888 commented on the issue:

https://github.com/apache/lucenenet/pull/188
  
@synhershko 

> I say let's first release, then see who uses this. Once we see a lot of 
usage, we may reconsider the implementation. WDYT?

In that case this is ready to go. But if we can spend 2-3 hours copying 
over an existing implementation that is almost a drop-in replacement for what 
we need, why not do one better?

Personally, I have a stake in faceted search - heck I have a whole open 
source project [BoboBrowse.Net](https://github.com/NightOwl888/BoboBrowse.Net), 
and I would like to have alternatives.

Also, Facet and Suggest are both brand new things to Lucene.Net. If you 
want to sway people's attention to this project, give them features they don't 
already have in Lucene.Net 3.0.3 (and make them as good as they can be). I have 
seen a few questions on the user mailing list from people who were trying to 
get this to work. Why not give them something useful?

> Let's have this discussion in the dev@ mailing list please. The list Shad 
provided may be correct, but our priorities are different - the spatial module 
needs work, and there are still failing tests at core. We can also skip 
Analysis.Kuromoji and Analysis.SmartCNand completely now - they aren't worth 
our efforts now. So, Let's have that discussion in the right place so other 
people could chime in as well.

Actually, I am nearly done with Analysis.Stempel now. I know it is not that 
critical, but it looked to be less than a day of work to finish and I started 
it before I knew whether these pull requests were going to be merged.

Also, nearly all of the "failures" in the core are due to the test runners 
screwing up. Many tests are in abstract classes that are meant to be run for 
multiple test subclasses. Those subclasses provide the setup that is necessary 
for the tests to run successfully. But, depending on what test runner you use, 
they are either being run without the setup *and* are being skipped or are 
being run *one additional time* in the abstract class without any setup, 
causing them to fail in the out-of-context run.

I know @conniey is currently working on setting up xUnit and I am hoping 
that its test runner(s) and/or specialized attributes are also a fix for this 
issue. If not, we need to find another solution that works regardless of the 
test runner used. The only thing I have come up with so far is to make those 
tests virtual, override them in the sub classes and only put the `[Test]` 
attribute on the stub methods in the sub classes, not in the abstract class. In 
fact, I did that in the Suggest tests before I realized how widespread this 
issue is (and that it also affects the QueryParser tests). It works, but 
creates a lot of duplicate stub methods that do nothing but call their base 
class.

And yes, I will continue this on the mailing list...

> On that note, @NightOwl888 I have no idea where you are located, but if I 
happen to be in your neighborhood during my travels do let me know and I'll buy 
you beers.

I am in Jomtien Beach, Thailand. And I'll take them draft, thank you :).




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] lucenenet pull request #187: Fixed 76 QueryParser Failing Tests

2016-10-02 Thread NightOwl888
Github user NightOwl888 commented on a diff in the pull request:

https://github.com/apache/lucenenet/pull/187#discussion_r81474715
  
--- Diff: src/Lucene.Net.QueryParser/Surround/Parser/QueryParser.cs ---
@@ -883,7 +883,7 @@ private void Jj_rescan_token()
 p = p.next;
 } while (p != null);
 }
-catch (LookaheadSuccess ls) { }
+catch (LookaheadSuccess /*ls*/) { }
 }
 jj_rescan = false;
 }
--- End diff --

Well, the alternative would be to use `#pragma` to disable the warning 
messages for the unused exception variable. Would you prefer it that way?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


Remaining Work/Priorities

2016-10-02 Thread Shad Storhaug
Hello,

I just wanted to open this discussion to talk about the work remaining to be 
done on Lucene.Net version 4.8.0. We are nearly there, but that doesn't mean we 
don't still need help!


FAILING TESTS
---

We now have over 5000 passing tests and as soon as pull request #188 
(https://github.com/apache/lucenenet/pull/188) is merged, by my count we have 
only 20 (actual) failing tests. Here is the breakdown by project:

Lucene.Net (Core) - 15 failing / 1989 total
Lucene.Net.Analysis.Common - 0 failing / 1445 total
Lucene.Net.Classification - 0 failing / 9 total
Lucene.Net.Expressions - 0 failing / 94 total
Lucene.Net.Facet - (including #188 will be) 0 failing / 152 total
Lucene.Net.Join - 0 failing / 27 total
Lucene.Net.Memory - 0 failing / 10 total
Lucene.Net.Misc - 2 failing / 42 total
Lucene.Net.Queries - 2 failing / 96 total
Lucene.Net.QueryParser - 1 failing / 203 total
Lucene.Net.Suggest - 0 failing / 142 total

The reason why I said ACTUAL tests above is because I recently discovered that 
many of the "failures" that are being reported are false negatives (in fact, 
the VS2015 NUnit test runner shows there are 135 failing tests total and 902 
tests total that don't belong to any project). Most NUnit 2.6 test runners do 
not correctly run tests in shared abstract classes with the correct context 
(test setup) to make them pass. These out-of-context runs add several 
additional minutes to the test run.

As an experiment, I upgraded to NUnit 3.4.1 and it helped the situation 
somewhat - that is, it ran the tests in the correct context and I was able to 
determine that we have more tests than the numbers above and they are all 
succeeding. However, it also ran the tests in an invalid context (that is, the 
context of the abstract class without any setup) and some of them still showed 
as failures.

I know @conniey is currently working on porting the tests over to xUnit. 
Hopefully, swapping test frameworks alone (or using some of the new fancy test 
attributes) is enough to fix this issue. If not, we need to find another 
solution - preferably one that can be applied to all of the tests in abstract 
classes without too much effort or changing them so they are too different from 
their Java counterpart.

Remaining Pieces to Port
-

I took an inventory of the remaining pieces left to port a few days ago and 
here is what that looks like (alphabetical order):

1. Analysis.ICU (Depends on ICU4j)
2. Analysis.Kuromoji
3. Analysis.Morfologik (Depends on Morfologik)
4. Analysis.Phonetic (Depends on Apache Commons)
5. Analysis.SmartCN
6. Analysis.Stempel (currently in progress)
7. Analysis.UIMA (Depends on Tagger, uimaj-core, WhiteSpaceTokenizer)
8. Benchmark (many dependencies)
9. Demo
10. Highlighter (Depends on Collator (which is still being ported) and 
BreakIterator (which we don't have a solution that works in .NET core yet))
11. Replicator (many dependencies)
12. Sandbox (Depends on Apache Jakarta)
13. Spatial (Already ported in #174 
(https://github.com/apache/lucenenet/pull/174), needs a recent version of 
spatial4n)
14. QueryParser.Flexible

Itamar, it would be helpful if you would be so kind as to organize this list in 
terms of priority. It also couldn't hurt to update the contributing documents 
(https://github.com/apache/lucenenet/blob/master/CONTRIBUTING.md, and 
https://cwiki.apache.org/confluence/display/LUCENENET/Current+Status with the 
latest information so anyone who wants to help out knows the current status.

Of course, it is the known status of dependencies that we need clarification 
on. Which of these dependencies is known to be ported? Which of them are ported 
but are not up to date? Which of them are known not to be ported, and which of 
them are unknown?


Public API Inconsistencies
-

One thing that I have had my eye on for a while now is the 
.NETification/consistency of the core API (that is, in the Lucene.Net project). 
There are several issues that I would like to address including:


1.   Method names that are still camelCase

2.   Properties that should be methods (because they do a lot of processing 
or because they are non-deterministic)

3.   Methods that should be properties

4.   .Size() vs .Size vs .Count - should generally all be .Count in .NET

5.   Interfaces should begin with "I"

6.   Classes should not begin with "I" followed by another capital letter 
(for some reason some of them were named that way)

7.   .CharAt() should probably be this[]

8.   Generic types nested within generic types (which cause Visual Studio 
to crash when Intellisense tries to read them)

... and so on. The only thing is these are all sweeping changes that will 
affect everyone helping out on Lucene.Net and anyone who is currently using the 
beta. So, I just wanted to gather some input on when the most appropriate time 
to begin working on these sweeping changes would be?