remove usage of Newtonsoft.Json by jetersen · Pull Request #2017 · GitTools/GitVersion

jetersen · 2020-01-03T14:48:40Z

fixes #2009

jetersen · 2020-01-03T15:03:08Z

src/GitVersionCore/BuildServers/AppVeyor.cs

-                name = $"GitVersion_{name}",
-                value = $"{value}"
-            };
+            var body = $"{{\"name\": \"GitVersion_{name}\",\"value\": \"{value}\"}}";


I doubt we need to evaluate whether value is an Integer or not.

Are you sure?

Before it was going through JsonConvert.SerializeObject as object with the value of value = $"{value}" ergo it was turned into strings. The serialized JSON is unchanged.

Good point. Let's tackle that in a separate PR, if at all.

jetersen · 2020-01-03T15:03:50Z

I see the appveyor actually tests the built DLL.

jetersen · 2020-01-03T15:08:50Z

@asbjornu unsure why docker tests failed.

arturcic · 2020-01-07T17:52:48Z

@Casz there are 2 more files to change, Directory.Build.props and test.props in the src folder

jetersen · 2020-01-07T18:06:34Z

There are some tests that I rather not rewrite I think it's fine that tests use newtonsoft.json 😄

jetersen · 2020-01-07T18:08:32Z

The problem that newtonsoft.json solves in tests is deserialize json to poco.

jetersen · 2020-01-07T18:14:48Z

@arturcic I can use system.text.json instead for the tests if you want?

Or I can use JavaScriptSerializer to deserialize as it is not performance critical.

arturcic · 2020-01-07T18:16:06Z

Sure works for me

jetersen · 2020-01-07T18:19:37Z

Hmmm that's odd system.text.json is already included... Looking into which package pulls it in 🤔

jetersen · 2020-01-07T18:28:30Z

Works for me:

jetersen · 2020-01-07T19:08:42Z

I am not sure how this ever worked, it seems that Newtonsoft.Json was filtering out none JSON objects from a string object.

I dare not touch...

This reverts commit b8b8858.

This reverts commit 21665b0.

jetersen · 2020-01-07T19:32:09Z

I see Newtonsoft.Json by default have no error handling setup.

So even if it receives an output string object like:

INFO [01/07/20 20:31:23:77] Applicable build agent found: 'TeamCity'.INFO [01/07/20 20:31:23:86] Working directory: C:\Users\josep\AppData\Local\Temp\TestRepositories\057e9db6-9c1a-4884-8ceb-89e8cc210785INFO [01/07/20 20:31:23:88] IsDynamicGitRepository: FalseINFO [01/07/20 20:31:23:92] Returning Project Root from DotGitDirectory: C:\Users\josep\AppData\Local\Temp\TestRepositories\057e9db6-9c1a-4884-8ceb-89e8cc210785\.git\ - C:\Users\josep\AppData\Local\Temp\TestRepositories\057e9db6-9c1a-4884-8ceb-89e8cc210785\INFO [01/07/20 20:31:23:92] Running on Windows.INFO [01/07/20 20:31:23:92] Applicable build agent found: 'TeamCity'.WARN [01/07/20 20:31:23:93] TeamCity doesn't make the current branch available through environmental variables.

Depending on your authentication and transport setup of your git VCS root things may work. In that case, ignore this warning.

In your TeamCity build configuration, add a parameter called `env.Git_Branch` with value %teamcity.build.vcs.branch.<vcsid>%

See http://gitversion.readthedocs.org/en/latest/build-server-support/build-server/teamcity for more infoINFO [01/07/20 20:31:23:93] Branch from build environment: INFO [01/07/20 20:31:23:93] Begin: Normalizing git directory for branch ''  INFO [01/07/20 20:31:23:98] One remote found (origin -> 'C:\git\code\GitVersion\src\GitVersionExe.Tests\bin\Debug\net472\TestRepositories\b58f239e-9bb0-41c2-9156-9160b6d64e8e').  INFO [01/07/20 20:31:23:98] Fetching from remote 'origin' using the following refspecs: +refs/pr/*/head:refs/remotes/origin/pr/*, +refs/heads/*:refs/remotes/origin/*.  INFO [01/07/20 20:31:24:01] Creating local branch from remote tracking 'refs/remotes/origin/FeatureBranch'.  INFO [01/07/20 20:31:24:04] End: Normalizing git directory for branch '' (Took: 110.00ms)  ERROR [01/07/20 20:31:24:18] An unexpected error occurred:
LibGit2Sharp.LibGit2SharpException: ref 'refs/remotes/origin/FeatureBranch' doesn't match the destination
   at LibGit2Sharp.Core.Ensure.HandleError(Int32 result)
   at LibGit2Sharp.Core.Proxy.git_refspec_rtransform(IntPtr refSpecPtr, String name)
   at LibGit2Sharp.Remote.FetchSpecTransformToSource(String reference)
   at LibGit2Sharp.BranchUpdater.GetUpstreamInformation(String canonicalName, String& remoteName, String& mergeBranchName)
   at LibGit2Sharp.BranchUpdater.SetUpstream(String upstreamBranchName)
   at GitVersion.Helpers.GitRepositoryHelper.<>c__DisplayClass8_1.<CreateOrUpdateLocalBranchesFromRemoteTrackingOnes>b__1(BranchUpdater b) in C:\git\code\GitVersion\src\GitVersionCore\Helpers\GitRepositoryHelper.cs:line 298
   at LibGit2Sharp.BranchCollection.Update(Branch branch, Action`1[] actions)
   at GitVersion.Helpers.GitRepositoryHelper.CreateOrUpdateLocalBranchesFromRemoteTrackingOnes(ILog log, Repository repo, String remoteName) in C:\git\code\GitVersion\src\GitVersionCore\Helpers\GitRepositoryHelper.cs:line 298
   at GitVersion.Helpers.GitRepositoryHelper.NormalizeGitDirectory(ILog log, IEnvironment environment, String gitDirectory, AuthenticationInfo authentication, Boolean noFetch, String currentBranch, Boolean isDynamicRepository) in C:\git\code\GitVersion\src\GitVersionCore\Helpers\GitRepositoryHelper.cs:line 41
   at GitVersion.GitPreparer.NormalizeGitDirectory(AuthenticationInfo auth, String targetBranch, String gitDirectory, Boolean isDynamicRepository) in C:\git\code\GitVersion\src\GitVersionCore\GitPreparer.cs:line 190
   at GitVersion.GitPreparer.Prepare(Boolean normalizeGitDirectory, String currentBranch, Boolean shouldCleanUpRemotes) in C:\git\code\GitVersion\src\GitVersionCore\GitPreparer.cs:line 62
   at GitVersion.GitVersionCalculator.CalculateVersionVariables() in C:\git\code\GitVersion\src\GitVersionCore\GitVersionCalculator.cs:line 52
   at GitVersion.ExecCommand.Execute() in C:\git\code\GitVersion\src\GitVersionExe\ExecCommand.cs:line 40
   at GitVersion.GitVersionExecutor.VerifyArgumentsAndRun(Arguments arguments) in C:\git\code\GitVersion\src\GitVersionExe\GitVersionExecutor.cs:line 117  INFO [01/07/20 20:31:24:18]   INFO [01/07/20 20:31:24:18] Attempting to show the current git graph (please include in issue):   INFO [01/07/20 20:31:24:18] Showing max of 100 commits  INFO [01/07/20 20:31:24:29] * 9bb62fb 52 minutes ago  (HEAD)
* 867dace 54 minutes ago  (origin/FeatureBranch, FeatureBranch)
* 9176cf2 56 minutes ago 
* b44da0d 58 minutes ago  (tag: 1.0.3, origin/master)

Such errors are completely ignored by Newtonsoft.Json.

asbjornu · 2020-01-08T13:29:53Z

How can such errors be ignored, @Casz? Newtonsoft.Json will throw regular Exceptions in the case of unparseable JSON, no?

jetersen · 2020-01-13T21:30:20Z

Still think this is mergable, I can cleanup the Newtonsoft.Json in tests in a follow up PR.

asbjornu

I think this looks good besides treating every value as string.

asbjornu · 2020-01-17T14:40:10Z

src/GitVersionCore/BuildServers/AppVeyor.cs

-                name = $"GitVersion_{name}",
-                value = $"{value}"
-            };
+            var body = $"{{\"name\": \"GitVersion_{name}\",\"value\": \"{value}\"}}";


Are you sure?

asbjornu · 2020-01-20T00:29:51Z

Hm. Is the serialization of integers related to #1688 in any way?

asbjornu · 2020-01-20T00:50:14Z

Thank you so much for your contributions, @Casz! 🙏

jetersen · 2020-01-20T05:00:43Z

@asbjornu Thank you for maintaining the project 🙇

I believe the JSON Output formatter does not check for null or empty.
I have #2019 I can fix it there, once I get back to it.

asbjornu · 2020-01-20T10:41:03Z

Sounds good, @Casz.

remove usage of Newtonsoft.Json

8d7099b

jetersen commented Jan 3, 2020

View reviewed changes

Merge branch 'master' into fix/newtonsoft.json-usage

adadf30

jetersen mentioned this pull request Jan 3, 2020

code cleanup: remove json output formatter #2019

Closed

asbjornu and others added 2 commits January 6, 2020 10:20

Merge branch 'master' into fix/newtonsoft.json-usage

c5f2df1

Merge branch 'master' into fix/newtonsoft.json-usage

d450d7c

arturcic added this to the 5.1.4 milestone Jan 7, 2020

arturcic added the improvement label Jan 7, 2020

jetersen added 2 commits January 7, 2020 19:31

remove all references to Newtonsoft.Json

21665b0

deserialize string dictionary

b8b8858

jetersen added 2 commits January 7, 2020 20:10

Revert "deserialize string dictionary"

d2f929e

This reverts commit b8b8858.

Revert "remove all references to Newtonsoft.Json"

7bd9aec

This reverts commit 21665b0.

Merge branch 'master' into fix/newtonsoft.json-usage

c48972f

asbjornu reviewed Jan 17, 2020

View reviewed changes

asbjornu approved these changes Jan 20, 2020

View reviewed changes

Merge branch 'master' into fix/newtonsoft.json-usage

ecc92e2

asbjornu merged commit d84c116 into GitTools:master Jan 20, 2020

jetersen mentioned this pull request Jan 23, 2020

add JSON Serializer #2060

Merged

Conversation

jetersen commented Jan 3, 2020

Uh oh!

jetersen Jan 3, 2020

Choose a reason for hiding this comment

Uh oh!

asbjornu Jan 17, 2020

Choose a reason for hiding this comment

Uh oh!

jetersen Jan 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asbjornu Jan 20, 2020

Choose a reason for hiding this comment

Uh oh!

jetersen commented Jan 3, 2020

Uh oh!

jetersen commented Jan 3, 2020

Uh oh!

arturcic commented Jan 7, 2020

Uh oh!

jetersen commented Jan 7, 2020

Uh oh!

jetersen commented Jan 7, 2020

Uh oh!

jetersen commented Jan 7, 2020

Uh oh!

arturcic commented Jan 7, 2020

Uh oh!

jetersen commented Jan 7, 2020

Uh oh!

jetersen commented Jan 7, 2020

Uh oh!

jetersen commented Jan 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jetersen commented Jan 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asbjornu commented Jan 8, 2020

Uh oh!

jetersen commented Jan 13, 2020

Uh oh!

asbjornu left a comment

Choose a reason for hiding this comment

Uh oh!

asbjornu Jan 17, 2020

Choose a reason for hiding this comment

Uh oh!

asbjornu commented Jan 20, 2020

Uh oh!

asbjornu commented Jan 20, 2020

Uh oh!

jetersen commented Jan 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asbjornu commented Jan 20, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jetersen Jan 17, 2020 •

edited

Loading

jetersen commented Jan 7, 2020 •

edited

Loading

jetersen commented Jan 7, 2020 •

edited

Loading

jetersen commented Jan 20, 2020 •

edited

Loading