Skip to content

TINKERPOP-2995 Create Sample Applications in each GLV#2298

Merged
Cole-Greer merged 54 commits intoapache:3.7-devfrom
Bit-Quill:ryan/glv-examples
Dec 14, 2023
Merged

TINKERPOP-2995 Create Sample Applications in each GLV#2298
Cole-Greer merged 54 commits intoapache:3.7-devfrom
Bit-Quill:ryan/glv-examples

Conversation

@ryn5
Copy link
Copy Markdown
Contributor

@ryn5 ryn5 commented Oct 17, 2023

https://issues.apache.org/jira/browse/TINKERPOP-2995
Created sample applications for each GLV in Java, Python, C#, JS, and Go which includes connection, basic Gremlin, and simple traversal examples.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Oct 17, 2023

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.15%. Comparing base (05ac05d) to head (d0d65e5).
⚠️ Report is 508 commits behind head on 3.7-dev.

Additional details and impacted files
@@              Coverage Diff              @@
##             3.7-dev    #2298      +/-   ##
=============================================
- Coverage      76.17%   76.15%   -0.02%     
+ Complexity     13113    13109       -4     
=============================================
  Files           1083     1083              
  Lines          64995    64995              
  Branches        7259     7259              
=============================================
- Hits           49510    49500      -10     
- Misses         12789    12794       +5     
- Partials        2696     2701       +5     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Member

@FlorianHockmann FlorianHockmann left a comment

Choose a reason for hiding this comment

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

I only reviewed the .NET example. Apart from my minor inline comments, I have two general questions:

  • Are the examples already included in our build pipeline? Because it looks to me like they aren't built via GH Actions.
  • How do we want to use the examples / How do we make users aware of them?

@ryn5
Copy link
Copy Markdown
Contributor Author

ryn5 commented Oct 19, 2023

@FlorianHockmann The examples aren't integrated into the pipeline or GH actions, but they are self-contained as simple projects in their respective example folders and can be run after building. Currently undecided on where to introduce them to users, but we could maybe add them to either the docs in each GLV section or in README files.

@FlorianHockmann
Copy link
Copy Markdown
Member

The examples aren't integrated into the pipeline or GH actions, but they are self-contained as simple projects in their respective example folders and can be run after building.

I think we should look into adding them to the main Maven build and at least add a smoke test for them (like executing the methods to ensure that no exception gets thrown) so they are covered by GH actions. Otherwise, we won't notice if we break them, e.g., by changing some API.
This probably means however that their connection parameters need to be changed as the test server doesn't listen on port 8182.

Currently undecided on where to introduce them to users, but we could maybe add them to either the docs in each GLV section or in README files.

I think a short mention in the reference docs for each GLV would be great as that is the place where we mostly document the GLVs. README files could of course also make sense, especially if the applications contain more than a single file (depending on @kenhuuu's comment) to briefly explain the application and its structure. README's are usually also what search engines will link to so that's where users will land if they didn't follow a link from somewhere else in our docs.

We should probably also remove the current Gremlin.Net.Template (docs src) as it basically served the same purpose and we don't need two example applications for .NET.

@spmallette
Copy link
Copy Markdown
Contributor

This probably means however that their connection parameters need to be changed as the test server doesn't listen on port 8182.

if we use a nonstandard port i think it would be good to ensure the comments are clear that the standard port is 8182.

@FlorianHockmann
Copy link
Copy Markdown
Member

Maybe I'm also overcautious here and we don't need any tests. Ensuring that the applications still compile probably already covers most cases as we would already notice breaking changes. But then I would at least let GH actions build the applications which should be easy to do by integrating them into the Maven build.

@kenhuuu
Copy link
Copy Markdown
Contributor

kenhuuu commented Nov 30, 2023

Maybe I'm also overcautious here and we don't need any tests. Ensuring that the applications still compile probably already covers most cases as we would already notice breaking changes. But then I would at least let GH actions build the applications which should be easy to do by integrating them into the Maven build.

This has already turned out to be a really good idea as #2366 would already cause these examples to break eventually.

@ryn5 ryn5 changed the base branch from master to 3.7-dev November 30, 2023 03:28
Copy link
Copy Markdown
Contributor

@Cole-Greer Cole-Greer left a comment

Choose a reason for hiding this comment

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

Thanks Ryan! Overall the changes here look good to me, just added a few minor comments.

@Cole-Greer
Copy link
Copy Markdown
Contributor

Thanks for all the updates Ryan!

VOTE +1

@vkagamlyk
Copy link
Copy Markdown
Contributor

VOTE+1

@Cole-Greer Cole-Greer merged commit 6fbafc4 into apache:3.7-dev Dec 14, 2023
@ryn5 ryn5 mentioned this pull request Feb 13, 2024
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.

7 participants