Skip to content

Only include STJ on net standard #3123

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

thompson-tomo
Copy link

@thompson-tomo thompson-tomo commented May 29, 2025

This PR adjusts the inclusion of System.Text.Json so that it is only included on net standard and not for the newer frameworks ie net 6.

The benefit is that it reduces the risk of conflicts which has occurred with other libraries, enables quicker remediation to CVE'S in STJ when end application deployed being dependent on framework as windows update can update the framework.

@andreachild
Copy link
Contributor

Hi @thompson-tomo thanks for submitting this change. Can you please provide a description for this PR according to the guide here: https://tinkerpop.apache.org/docs/current/dev/developer/#pull-requests

Please also add a changelog entry to describe the change you've made so that users of the .net driver can be made aware.

@codecov-commenter
Copy link

codecov-commenter commented Jun 2, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.18%. Comparing base (cfd6889) to head (77c390a).
Report is 265 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3123      +/-   ##
============================================
+ Coverage     77.87%   78.18%   +0.31%     
- Complexity    13578    13774     +196     
============================================
  Files          1015     1024       +9     
  Lines         59308    60075     +767     
  Branches       6835     7003     +168     
============================================
+ Hits          46184    46968     +784     
+ Misses        10817    10752      -65     
- Partials       2307     2355      +48     

☔ 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.

@Cole-Greer
Copy link
Contributor

Hi @thompson-tomo, I'm curious what is the motivation for this change. I'm not aware of any compatibility issues here, but .Net is also outside of my area of expertise.

@thompson-tomo
Copy link
Author

@Cole-Greer i have run in to issues with other libraries. The motivation is to proactively reduce the chance of issues/conflicts while also improving the ability to quickly remediate CVE'S in the field as the responsibility for managing the affected library falls to the framework rather than the consuming application

@andreachild
Copy link
Contributor

VOTE +1

3 similar comments
@EchoNullify
Copy link
Contributor

VOTE +1

@Cole-Greer
Copy link
Contributor

VOTE +1

@xiazcy
Copy link
Contributor

xiazcy commented Jun 13, 2025

VOTE +1

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.

6 participants