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

Replace Newtonsoft.Json package with System.Text.Json #1513

Merged
merged 19 commits into from
Sep 10, 2023

Conversation

Bertk
Copy link
Collaborator

@Bertk Bertk commented Aug 4, 2023

❗ PR #1510 should be merged before this one. The System.Text.Json updates shall be merges separately.

@Bertk Bertk force-pushed the try-system-text-json branch 2 times, most recently from 6c06d39 to f8dcd31 Compare August 7, 2023 14:31
@Bertk Bertk marked this pull request as ready for review August 7, 2023 15:33
@Bertk
Copy link
Collaborator Author

Bertk commented Aug 8, 2023

@daveMueller @MarcoRossignoli
This PR replaces Newtonsoft.Json from coverlet.core with System.Text.Json and removes many assemblies from the nuget packages. Please share your thoughts on this.

image

@MarcoRossignoli
Copy link
Collaborator

That's really cool, @daveMueller do you plan to review this one?

@daveMueller
Copy link
Collaborator

Yes I will. As I understood it's the same changes as in #1510 but instead of updating Newtonsoft.Jsonit uses System.Text.Json.

@Bertk
Copy link
Collaborator Author

Bertk commented Aug 10, 2023

I will rebase this branch after #1510 is merged 😉

@Bertk Bertk force-pushed the try-system-text-json branch 2 times, most recently from c9e23de to 16cdd64 Compare August 11, 2023 08:28
@MarcoRossignoli
Copy link
Collaborator

@Bertk I see you a lot active on coverlet, let me know if you're interested to become a co-maintainer and help to keep the product in a good shape.

You can contact me on twitter https://twitter.com/MarcoRossignoli or let me know how we can have a chat

Not mandatory at all it's OSS so no obligation.

cc: @daveMueller @petli @tonerdo

@Bertk
Copy link
Collaborator Author

Bertk commented Aug 11, 2023

@MarcoRossignoli Can we have a chat on Monday Zoom or MS Teams will be fine.

@Bertk Bertk changed the title Try system text json Replace Newtonsoft.Json package with System.Text.Json Aug 17, 2023
@Bertk Bertk force-pushed the try-system-text-json branch from 16cdd64 to aabfdc3 Compare August 20, 2023 06:11
@Bertk
Copy link
Collaborator Author

Bertk commented Aug 20, 2023

@daveMueller @MarcoRossignoli Please review and approve PR.

Please note: I wanted to check stderr with the last commit but after the update the CI tests were successful.

By the way, the obsolete PR #1358 should be closed.

@Bertk Bertk removed the request for review from MarcoRossignoli August 26, 2023 12:49
@Bertk Bertk force-pushed the try-system-text-json branch 2 times, most recently from 51c6d81 to 49afd1b Compare August 26, 2023 20:16
@Bertk Bertk force-pushed the try-system-text-json branch from 49afd1b to 0f1c051 Compare September 3, 2023 06:16
@Bertk
Copy link
Collaborator Author

Bertk commented Sep 3, 2023

@daveMueller Please review PR. This has also the code coverage report for the AzureDevOps build pipeline.

@Bertk Bertk force-pushed the try-system-text-json branch 3 times, most recently from 6893fb0 to c3710a9 Compare September 5, 2023 09:43
@Bertk Bertk force-pushed the try-system-text-json branch from c3710a9 to b4ad03d Compare September 5, 2023 09:51
@Bertk
Copy link
Collaborator Author

Bertk commented Sep 5, 2023

@daveMueller I added code coverage badge and trx files which have the output attachments. The PR is ready for merge.

Copy link
Collaborator

@daveMueller daveMueller left a comment

Choose a reason for hiding this comment

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

Thanks @Bertk 👍

@daveMueller daveMueller merged commit 2307f65 into coverlet-coverage:master Sep 10, 2023
@Bertk Bertk deleted the try-system-text-json branch September 11, 2023 05:09
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