Skip to content

Conversation

@brettfo
Copy link
Member

@brettfo brettfo commented Mar 8, 2018

Follow up to #4464.

When the VSIX packages were converted to be SDK-style projects in #4417 Newtonsoft.Json.dll stopped getting added to VisualFSharpFull.vsix/VisualFSharpOpenSource.vsix due to shortcomings in the VSSDK.

The fix is to manually add that file to the <VSIXSourceItem> item group right before the VSIX container is created.

@Pilchie
Copy link
Member

Pilchie commented Mar 8, 2018

Do we actually need this? Shouldn't it be on the VS binding path elsewhere and we should ensure that we're using the same version as the rest of VS?

@AArnott - do you have guidance for how to depend on Newtonsoft.Json?

@AArnott
Copy link

AArnott commented Mar 8, 2018

@Pilchie is right. Please do not ship newtonsoft.json with any extension. That's likely why you found you had to hack around the vs SDK: we're trying to help you do the right thing by default. Why do you find you need to ship it? Are you shipping a version newer than the one that vs ships? If so please retarget to the one that matches the version vs ships.

Copy link

@AArnott AArnott left a comment

Choose a reason for hiding this comment

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

Don't do this. Target a different version of newtonsoft if you need to instead.

@brettfo
Copy link
Member Author

brettfo commented Mar 8, 2018

We can certainly retarget to 9.0.1 (what's currently in 15.6), but how do we get notified when the VS version changes so we can stay in sync?

@Pilchie
Copy link
Member

Pilchie commented Mar 8, 2018

You shouldn't have to in general - if VS updates, it will update binding redirects, and since the library won't have breaking changes, your code should just work.

@AArnott
Copy link

AArnott commented Mar 8, 2018

What @Pilchie said is usually true. However in this particular case, 9.0.1 is the last version without breaking changes. So when (and if) we update VS to bundle 10 or later, we may not add binding redirects, but in that case we'll ship both 9 and 10 to avoid breaking folks.

@brettfo
Copy link
Member Author

brettfo commented Mar 9, 2018

Closing this in favor of #4479.

@brettfo brettfo closed this Mar 9, 2018
@brettfo brettfo deleted the vsix-newtonsoft-json branch March 9, 2018 22:47
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