Reference loop => JSON.NET 5.0.6 #451

Merged
merged 5 commits into from Jul 25, 2013

Projects

None yet

2 participants

@dahlbyk
Contributor
dahlbyk commented Jul 10, 2013

IgnoreEnumerableReferenceLoop fails before JSON.NET upgrade.

I also wonder if it makes sense to set JsonNetSerializer.Settings.ReferenceLoopHandling = ReferenceLoopHandling.Ignore, since you're really just serializing for informational purposes. It doesn't really seem (to me) that it's an "error" worth reporting.

@dahlbyk
Contributor
dahlbyk commented Jul 10, 2013

I'm not sure why the build is timing out... Works On My Machine™

@avanderhoorn
Member

Did you commit all files? I don't see the JsonNetSerializer.Settings.ReferenceLoopHandling = ReferenceLoopHandling.Ignore line anywhere in the commits?

@dahlbyk
Contributor
dahlbyk commented Jul 10, 2013

I did not include that change (tests should pass without it), just thought I would include the suggestion. I can add if you'd like.

@dahlbyk
Contributor
dahlbyk commented Jul 23, 2013

Tried --amend and push --force...seems like it's going to fail again.

The build is stalling here:
http://teamcity.codebetter.com/viewLog.html?buildId=79704&buildTypeId=bt428&tab=buildLog#_focus=2970

Here's the corresponding line for a successful build:
http://teamcity.codebetter.com/viewLog.html?buildId=80253&buildTypeId=bt428&tab=buildLog#_focus=2959

In short, ilmerge behavior for Glimpise.Core.Net45 has changed. I'm going to try changing the places that reference targetFramework="net45" back to net40.

@dahlbyk
Contributor
dahlbyk commented Jul 23, 2013

@JamesNK, have you heard of anything weird related to ILMerge and your net45 build? Things got weird on the build server (but not on my machine) until I reverted to reference the net40 build instead.

@dahlbyk
Contributor
dahlbyk commented Jul 23, 2013

I also wonder if it makes sense to set JsonNetSerializer.Settings.ReferenceLoopHandling = ReferenceLoopHandling.Ignore, since you're really just serializing for informational purposes. It doesn't really seem (to me) that it's an "error" worth reporting.

This change is now included.

dahlbyk added some commits Jul 23, 2013
@dahlbyk dahlbyk Use object initializer 84c73e5
@dahlbyk dahlbyk Add failing ReferenceLoop tests 5b65b65
@dahlbyk dahlbyk Enable test that JSON serialize error is logged 7091ced
@dahlbyk dahlbyk Upgrade to Json.NET 5.0.6
Note that this version of Json.NET does ship with a net45 version,
but it causes problems with ILMerge. Resolved by referencing net40.

See #451 for discussion.
e4ec9ab
@dahlbyk dahlbyk Set Json.NET ReferenceLoopHandling = Ignore 0410160
@dahlbyk
Contributor
dahlbyk commented Jul 24, 2013

Cleaned up and rebased on latest.

@avanderhoorn

Didn't you like the way it was ;)

Owner

No 😁

@avanderhoorn
Member

Glad to have your commit!

@avanderhoorn avanderhoorn merged commit 85ed0f9 into Glimpse:master Jul 25, 2013

1 check passed

default TeamCity Build Glimpse :: Continuous Integration finished: Tests passed: 949, ignored: 10
Details
@avanderhoorn
Member

For the record what was the issue with the build again?

@dahlbyk
Contributor
dahlbyk commented Jul 26, 2013

The build was stalling during the ILMerge of the net45 version (but it worked on my machine). Switching to the net40 version of Json.NET fixed the issue, but it's unclear if it's an ILMerge bug or something with Json.NET's (new since Glimpse's last version) net45 build.

@CGijbels CGijbels added a commit that referenced this pull request Jul 30, 2013
@dahlbyk @CGijbels dahlbyk + CGijbels Upgrade to Json.NET 5.0.6
Note that this version of Json.NET does ship with a net45 version,
but it causes problems with ILMerge. Resolved by referencing net40.

See #451 for discussion.
143c43f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment