Re: [PR] TINKERPOP-3029 Fix enumeration for .NET 8 [tinkerpop]
EricSites commented on code in PR #2424:
URL: https://github.com/apache/tinkerpop/pull/2424#discussion_r1441072113
##
gremlin-dotnet/src/Gremlin.Net/Process/Traversal/DefaultTraversal.cs:
##
@@ -82,21 +82,22 @@ public bool MoveNext()
}
private bool MoveNextInternal()
-{
+{
if (_fetchedNext) return _nextAvailable;
+
+if (!_nextAvailable || _nextAvailable &&
TraverserEnumerator.Current?.Bulk == 0)
Review Comment:
This code will also throw an exception if called without first starting the
iterator: You will need to track that the iterator has not been started.
```.csharp
private object GetCurrent()
{
// Use dynamic to object to prevent runtime dynamic conversion evaluation
return TraverserEnumerator.Current?.Object;
}
```
Please review the new [.Net code
here](https://source.dot.net/#PresentationFramework/MS/Internal/WindowsRuntime/Generated/WinRT/Projections/IEnumerable.cs,239)
--
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: commits-unsubscr...@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] TINKERPOP-3029 Fix enumeration for .NET 8 [tinkerpop]
vkagamlyk commented on code in PR #2424:
URL: https://github.com/apache/tinkerpop/pull/2424#discussion_r1440829828
##
gremlin-dotnet/src/Gremlin.Net/Process/Traversal/DefaultTraversal.cs:
##
@@ -82,21 +82,22 @@ public bool MoveNext()
}
private bool MoveNextInternal()
-{
+{
if (_fetchedNext) return _nextAvailable;
+
+if (!_nextAvailable || _nextAvailable &&
TraverserEnumerator.Current?.Bulk == 0)
Review Comment:
can be simplified
```suggestion
if (!_nextAvailable || TraverserEnumerator.Current?.Bulk == 0)
```
--
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: commits-unsubscr...@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] TINKERPOP-3029 Fix enumeration for .NET 8 [tinkerpop]
codecov-commenter commented on PR #2424: URL: https://github.com/apache/tinkerpop/pull/2424#issuecomment-1875641877 ## [Codecov](https://app.codecov.io/gh/apache/tinkerpop/pull/2424?src=pr=h1_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) Report All modified and coverable lines are covered by tests :white_check_mark: > Comparison is base [(`e86eed2`)](https://app.codecov.io/gh/apache/tinkerpop/commit/e86eed20a868d5ba8fcf64939f032b87093811c1?el=desc_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) 75.16% compared to head [(`76df767`)](https://app.codecov.io/gh/apache/tinkerpop/pull/2424?src=pr=desc_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) 71.23%. Additional details and impacted files ```diff @@ Coverage Diff @@ ## 3.6-dev#2424 +/- ## = - Coverage 75.16% 71.23% -3.93% = Files 1057 25-1032 Lines 63470 3772 -59698 Branches69360-6936 = - Hits 47706 2687 -45019 + Misses 13191 898 -12293 + Partials2573 187-2386 ``` [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/tinkerpop/pull/2424?src=pr=continue_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache). :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache). -- 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: commits-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
(tinkerpop) branch TINKERPOP-3029 created (now 76df767fb0)
This is an automated email from the ASF dual-hosted git repository. florianhockmann pushed a change to branch TINKERPOP-3029 in repository https://gitbox.apache.org/repos/asf/tinkerpop.git at 76df767fb0 TINKERPOP-3029 Fix enumeration for .NET 8 This branch includes the following new commits: new 76df767fb0 TINKERPOP-3029 Fix enumeration for .NET 8 The 1 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference.
(tinkerpop) 01/01: TINKERPOP-3029 Fix enumeration for .NET 8
This is an automated email from the ASF dual-hosted git repository.
florianhockmann pushed a commit to branch TINKERPOP-3029
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit 76df767fb0fcf5ef42fb7b4b249d6d65d548a30f
Author: Florian Hockmann
AuthorDate: Wed Jan 3 17:07:45 2024 +0100
TINKERPOP-3029 Fix enumeration for .NET 8
`IEnumerable.Current` has been changed in .NET 8. Before .NET 8, it
simply returned `null` if `MoveNext()` wasn't called first. With .NET 8
however it throws an exception.
Since this has apparently already been undefined behavior before, we
should fix this irrespective of .NET 8:
https://github.com/dotnet/runtime/issues/85243#issuecomment-1521085177
The problem can be reproduced by executing the Gherkin tests with .NET
without the change in `DefaultTraversal` (by changing the
`TargetFramework` in `Gremlin.Net.IntegrationTest.csproj` to `net8.0`).
.NET 8 needs to be installed for this of course.
---
.../Gremlin.Net/Process/Traversal/DefaultTraversal.cs | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git
a/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/DefaultTraversal.cs
b/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/DefaultTraversal.cs
index 191b7a8293..f8a2286f19 100644
--- a/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/DefaultTraversal.cs
+++ b/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/DefaultTraversal.cs
@@ -82,21 +82,22 @@ namespace Gremlin.Net.Process.Traversal
}
private bool MoveNextInternal()
-{
+{
if (_fetchedNext) return _nextAvailable;
+
+if (!_nextAvailable || _nextAvailable &&
TraverserEnumerator.Current?.Bulk == 0)
+{
+_nextAvailable = TraverserEnumerator.MoveNext();
+}
+if (!_nextAvailable) return false;
var currentTraverser = TraverserEnumerator.Current;
-if (currentTraverser?.Bulk > 1)
+if (currentTraverser?.Bulk >= 1)
{
currentTraverser.Bulk--;
-_nextAvailable = true;
-}
-else
-{
-_nextAvailable = TraverserEnumerator.MoveNext();
}
-return _nextAvailable;
+return true;
}
///
