You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucenenet.apache.org by "J-Exodus (via GitHub)" <gi...@apache.org> on 2024/07/25 15:07:12 UTC
[PR] Completed work for Issue #916 - Conversion to native Array.Empty() [lucenenet]
J-Exodus opened a new pull request, #953:
URL: https://github.com/apache/lucenenet/pull/953
Conversion to native Array.Empty<T>()
Fixes #916
## Description
This is my first real PR. Always happy for feedback.
- Removed ```Lucene.Net.Support.Arrays.Empty<T>()``` and ```FEATURE_ARRAYEMPTY```.
- Converted all ```Arrays.Empty<T>()``` to ```Array.Empty<T>()``` and added ```using System;``` where required.
- Removed ```using Lucene.Net.Support;``` statements that were no longer required.
- In ```FieldConparitor.cs``` converted ```MISSING_BTYES``` and ```NON_MISSING_BYTES``` to ```Array.Empty<bytes>()```
Conducted unit tests - all passed. Unsure correct format to attach results.
Unfortunately, my linter has picked up the extra white spaces in the files I updated, which are marked as individual changes. If this effects the ability to review and merge this PR, please let me know and I'll redo it all with the linter off.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Completed work for Issue #916 - Conversion to native Array.Empty() [lucenenet]
Posted by "NightOwl888 (via GitHub)" <gi...@apache.org>.
NightOwl888 commented on code in PR #953:
URL: https://github.com/apache/lucenenet/pull/953#discussion_r1713114757
##########
src/Lucene.Net.Analysis.Common/Analysis/Query/QueryAutoStopWordAnalyzer.cs:
##########
@@ -3,10 +3,10 @@
using Lucene.Net.Analysis.Core;
using Lucene.Net.Analysis.Util;
using Lucene.Net.Index;
-using Lucene.Net.Support;
using Lucene.Net.Util;
using System.Collections.Generic;
using System.IO;
+using System;
Review Comment:
Please place this before other `System` usings. We keep these in lexocographical sort order.
##########
src/Lucene.Net/Search/FieldComparator.cs:
##########
@@ -1455,14 +1455,10 @@ public override int CompareValues(BytesRef val1, BytesRef val2)
// TODO: should we remove this? who really uses it?
public sealed class TermValComparer : FieldComparer<BytesRef>
{
- // LUCENENET NOTE: We can't use Arrays.Empty<byte>() here because
Review Comment:
Please revert this change and replace this comment. These are sentinel values that are compared for reference equality. Making them the same instance causes the TestSort.TestStringValMissingVsNotMissing() test to fail. This bug was fixed in #912, but this breaks it once again.
Reverting this entire file back to its original state would be an acceptable fix.
##########
src/Lucene.Net.Suggest/Suggest/SortedInputIterator.cs:
##########
@@ -1,9 +1,9 @@
using Lucene.Net.Store;
-using Lucene.Net.Support;
using Lucene.Net.Support.IO;
using Lucene.Net.Util;
using System.Collections.Generic;
using System.IO;
+using System;
Review Comment:
Please place this line above `using System.Collections.Generic;`.
##########
src/dotnet/tools/Lucene.Net.Tests.Cli/Commands/Index/IndexSplitCommandTest.cs:
##########
@@ -3,6 +3,7 @@
using NUnit.Framework;
using System.Collections.Generic;
using System.Linq;
+using System;
Review Comment:
Please move this line above `using System.Collections.Generic;`.
##########
src/dotnet/tools/Lucene.Net.Tests.Cli/Commands/Index/IndexFixCommandTest.cs:
##########
@@ -1,9 +1,9 @@
using Lucene.Net.Attributes;
using Lucene.Net.Cli.CommandLine;
-using Lucene.Net.Support;
using NUnit.Framework;
using System.Collections.Generic;
using System.Linq;
+using System;
Review Comment:
Please move this line above `using System.Collections.Generic;`.
##########
src/Lucene.Net.Expressions/JS/JavascriptLexer.cs:
##########
@@ -786,21 +785,21 @@ public void MNAMESPACE_ID()
switch (alt1)
{
case 1:
- {
Review Comment:
Please revert all indentation and other formatting changes in this file. It is time-consuming to go through many changes to find the functional lines that actually changed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Completed work for Issue #916 - Conversion to native Array.Empty() [lucenenet]
Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin merged PR #953:
URL: https://github.com/apache/lucenenet/pull/953
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Completed work for Issue #916 - Conversion to native Array.Empty() [lucenenet]
Posted by "J-Exodus (via GitHub)" <gi...@apache.org>.
J-Exodus commented on PR #953:
URL: https://github.com/apache/lucenenet/pull/953#issuecomment-2286270042
Thanks for your patience in providing feedback for this PR. I have corrected the issues as identified. This one should now be good to go.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org