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

Start trying to remove graph api sdk dependency #1003

Draft
wants to merge 28 commits into
base: dev
Choose a base branch
from

Conversation

quails4Eva
Copy link

Reduce dependency on Microsoft.Graph NuGet package
Based on discussion in #660
#660 (comment)

var subscriptionModel = MapGraphEntityToModel(subscription);
return subscriptionModel;
}).GetAwaiter().GetResult();
var response = JToken.Parse(responseAsString);
Copy link
Author

Choose a reason for hiding this comment

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

Converting string to Model seems like it could be an extension method?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also tagging @KoenZomers as he wrote a lot of the original code

@jansenbe jansenbe self-assigned this Apr 10, 2024
var subscriptionModel = MapGraphEntityToModel(subscription);
return subscriptionModel;
}).GetAwaiter().GetResult();
var response = JToken.Parse(responseAsString);
Copy link
Contributor

@jansenbe jansenbe Apr 11, 2024

Choose a reason for hiding this comment

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

@quails4Eva : would recommend using System.Text.Json over Newtonsoft

Copy link
Author

Choose a reason for hiding this comment

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

I think I've done this. I copied the newtonsoft code from elsewhere in the project, but I think this should be equivalent for system.text.json . It needed the package version updating for .net standard. I've changed it to use the same version for each target framework as I don't think system.text.json is tied to framework versions as some other packages are

@quails4Eva
Copy link
Author

Next time I have some free time to look at this I'll try and replace the other usages in SubscriptionsUtility

@quails4Eva
Copy link
Author

I started adding some async http bits to maintain where async was already being used, but then I realised that the async usage was all wrapped in Task.Run so probably not really helping anything. I can undo that if you'd rather it be put back.

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.

None yet

2 participants