Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix doc links and update test naming #94

Open
wants to merge 1 commit into
base: 3.7-dev
Choose a base branch
from

Conversation

xiazcy
Copy link

@xiazcy xiazcy commented Nov 16, 2023

No description provided.

Copy link

codecov bot commented Nov 17, 2023

Codecov Report

Attention: 18 lines in your changes are missing coverage. Please review.

Comparison is base (515ff5b) 76.18% compared to head (d8ddbfd) 76.16%.
Report is 4 commits behind head on 3.7-dev.

Files Patch % Lines
...mlin/driver/handler/HttpGremlinRequestEncoder.java 0.00% 8 Missing ⚠️
...p/gremlin/server/handler/HttpUserAgentHandler.java 66.66% 3 Missing and 3 partials ⚠️
...in/server/handler/WsAndHttpChannelizerHandler.java 71.42% 1 Missing and 1 partial ⚠️
...g/apache/tinkerpop/gremlin/driver/Channelizer.java 0.00% 1 Missing ⚠️
...kerpop/gremlin/server/channel/HttpChannelizer.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             3.7-dev      #94      +/-   ##
=============================================
- Coverage      76.18%   76.16%   -0.02%     
+ Complexity     13109    13107       -2     
=============================================
  Files           1082     1083       +1     
  Lines          64967    64994      +27     
  Branches        7252     7256       +4     
=============================================
+ Hits           49496    49504       +8     
- Misses         12780    12791      +11     
- Partials        2691     2699       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kenhuuu
Copy link

kenhuuu commented Nov 17, 2023

LGTM. Seems mostly like formatting changes.

@@ -1543,7 +1543,7 @@ Null values from the incoming traverser are not processed and remain as null whe
* For `Scope.global` or parameterless function calls, if the incoming traverser is a non-String value then an `IllegalArgumentException` will be thrown.
* For `Scope.local`, if the incoming traverser is not a string or a list of strings then an `IllegalArgumentException` will be thrown.

See: link:https://github.com/apache/tinkerpop/tree/x.y.z/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/ToLowerStep.java[source],
See: link:https://github.com/apache/tinkerpop/tree/x.y.z/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/ToLowerGlobalStep.java[source],

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably add a second link for the local versions of all of these steps as well.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dedup only links to the global one, so I'm leaving it as the global one (since it's also the default)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I wrote the dedup semantics docs, I think that was an oversight on my part

@@ -341,7 +341,7 @@ See: link:https://tinkerpop.apache.org/docs/x.y.z/reference/#dateAdd-step[dateAd
See: link:https://tinkerpop.apache.org/docs/x.y.z/reference/#dateDiff-step[dateDiff()-step]
See: link:https://issues.apache.org/jira/browse/TINKERPOP-2979[TINKERPOP-2979]

===== `datetime()` for current server time
===== `datetime()` for Current Server Time

Function `datetime()` extended to return current server time when used without argument.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to add a note that datetime() remains a script only function? Or is it best to leave that out?

@xiazcy xiazcy force-pushed the yang/fix-doc-and-test-name branch from bd83613 to 9c4022d Compare November 17, 2023 01:36
@xiazcy xiazcy force-pushed the yang/fix-doc-and-test-name branch from 4dd0996 to d8ddbfd Compare November 17, 2023 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants