You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by "porunov (via GitHub)" <gi...@apache.org> on 2023/06/14 23:41:24 UTC

[GitHub] [tinkerpop] porunov opened a new pull request, #2097: Remove final class declaration for LabelStep

porunov opened a new pull request, #2097:
URL: https://github.com/apache/tinkerpop/pull/2097

   In JanusGraph we would like to make some optimizations to `LabelStep` (https://github.com/JanusGraph/janusgraph/issues/3813).
   
   We need to extend `LabelStep` with additional logic. Even so it's quite simple to implement our own `LabelStep`, it's a little bit dangerous because TinkerPop's `LabelStep` may be used in some optimization strategies (including user-provided optimization strategies). If we replace `LabelStep` with something which doesn't extend `LabelStep` then we may risk breaking some strategies.
   
   For example, here are the places in TinkerPop where `step instanceof LabelStep` is checked:
   1) https://github.com/apache/tinkerpop/blob/f00cfe5592ef45e64fb45737a16ec5877a824958/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/ByModulatorOptimizationStrategy.java#L98
   2) https://github.com/apache/tinkerpop/blob/f00cfe5592ef45e64fb45737a16ec5877a824958/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/traversal/MasterExecutor.java#L149
   3) https://github.com/apache/tinkerpop/blob/f00cfe5592ef45e64fb45737a16ec5877a824958/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/TraversalHelper.java#L106
   
   Thus, I propose to remove `final` class declaration for `LabelStep`, so that graph providers could extend this step.
   The step looks fairly stable.
   
   Related issue: https://issues.apache.org/jira/browse/TINKERPOP-2927


-- 
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-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tinkerpop] codecov-commenter commented on pull request #2097: Remove final class declaration for LabelStep

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #2097:
URL: https://github.com/apache/tinkerpop/pull/2097#issuecomment-1592140076

   ## [Codecov](https://app.codecov.io/gh/apache/tinkerpop/pull/2097?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   > Merging [#2097](https://app.codecov.io/gh/apache/tinkerpop/pull/2097?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (d934129) into [3.6-dev](https://app.codecov.io/gh/apache/tinkerpop/commit/f00cfe5592ef45e64fb45737a16ec5877a824958?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (f00cfe5) will **decrease** coverage by `5.03%`.
   > The diff coverage is `n/a`.
   
   ```diff
   @@              Coverage Diff              @@
   ##             3.6-dev    #2097      +/-   ##
   =============================================
   - Coverage      75.22%   70.20%   -5.03%     
   =============================================
     Files           1047       24    -1023     
     Lines          62690     3500   -59190     
     Branches        6864        0    -6864     
   =============================================
   - Hits           47161     2457   -44704     
   + Misses         12985      876   -12109     
   + Partials        2544      167    -2377     
   ```
   
   
   [see 1024 files with indirect coverage changes](https://app.codecov.io/gh/apache/tinkerpop/pull/2097/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_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-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tinkerpop] Cole-Greer merged pull request #2097: Remove final class declaration for LabelStep

Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer merged PR #2097:
URL: https://github.com/apache/tinkerpop/pull/2097


-- 
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-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tinkerpop] vkagamlyk commented on pull request #2097: Remove final class declaration for LabelStep

Posted by "vkagamlyk (via GitHub)" <gi...@apache.org>.
vkagamlyk commented on PR #2097:
URL: https://github.com/apache/tinkerpop/pull/2097#issuecomment-1599623696

   VOTE +1


-- 
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-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tinkerpop] lyndonbauto commented on pull request #2097: Remove final class declaration for LabelStep

Posted by "lyndonbauto (via GitHub)" <gi...@apache.org>.
lyndonbauto commented on PR #2097:
URL: https://github.com/apache/tinkerpop/pull/2097#issuecomment-1599427496

   lgtm VOTE+1


-- 
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-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tinkerpop] Cole-Greer commented on pull request #2097: Remove final class declaration for LabelStep

Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on PR #2097:
URL: https://github.com/apache/tinkerpop/pull/2097#issuecomment-1633033436

   VOTE +1


-- 
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-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org