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

Upgrade Newtonsoft.Json and NQuads parsing fixes #14

Merged
merged 18 commits into from
May 18, 2016

Conversation

tpluscode
Copy link
Collaborator

Changes in Newtonsoft.Json >=6 break conversion from JSON-LD to RDF.
I'm also re-submitting the culture-specific fixes

@dnfclas
Copy link

dnfclas commented Mar 25, 2016

Hi @tpluscode, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla2.dotnetfoundation.org.

TTYL, DNFBOT;

@dnfclas
Copy link

dnfclas commented Mar 25, 2016

@tpluscode, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

@tpluscode tpluscode changed the title Upgrade Newtonsoft.Json Upgrade Newtonsoft.Json and NQuads parsing fixes Mar 28, 2016
@asbjornu
Copy link
Member

@tpluscode As this pull request is close to 2 months old with no reaction from a human being and the repository hasn't been touched by a human for 1.5 years, how do you feel about my idea in #13 to fork this baby, so we can get things moving?

@tpluscode
Copy link
Collaborator Author

tpluscode commented May 13, 2016

Yes, this is a very disappointing turn of events. I'm not sure how this is going to work out. Would you publish an alternative package? Or just ship binaries with Nancy.RDF, possibly IL merged? Or maybe publish the package with same id on MyGet?

@asbjornu
Copy link
Member

asbjornu commented May 13, 2016

Publishing with the same ID is impossible, I think. Only maintainers of the existing package (@sblom and @NuGet) can upload packages with the same ID. But we could either set up a public MyGet feed, mint a new ID or just package it with Nancy.Rdf.

A new ID for the package and name for the forked repository could be LinkedDataSharp, LinkedData.NET or something similar.

foreach (var c in ((string) input))
{
if (c == ':')
hasColon = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

style: add { }'s around this

@emgarten
Copy link
Contributor

@tpluscode thanks for the PR, it's good to see this library getting some help 😄

The changes and extra tests look good to me. :shipit:

@asbjornu
Copy link
Member

@emgarten Yay, a human! 😄 What's the chance of getting this PR merged and released on NuGet? Can you help out with that?

@emgarten
Copy link
Contributor

@asbjornu I'll take a look at building the nupkg for this with the changes next week. @johnataylor is going to review this also.

@asbjornu
Copy link
Member

@emgarten @johnataylor Brilliant! 😄 👍

@emgarten emgarten merged commit 5bdd4e8 into linked-data-dotnet:master May 18, 2016
@emgarten
Copy link
Contributor

I've merged the changes and created a 1.0.5 package here: https://www.myget.org/F/jsonld/api/v2/package/JsonLD/1.0.5

@asbjornu @tpluscode would you try out the new package and verify that it works for you? If it does I'll push it to nuget.org.

@tpluscode tpluscode deleted the upgrade-newtonsoft branch May 20, 2016 14:09
@tpluscode
Copy link
Collaborator Author

@emgarten I've updated and it seems to work fine.

Did you change the package name on purpose though? On NuGet it's json-ld.net and on MyGet it's JsonLd. I'd rather your didn't change that name.

@emgarten
Copy link
Contributor

@tpluscode thanks for verifying and all the work on the PR, this is a great improvement 😄

I'll change before pushing this to nuget.org. I'm going to add a build script to this repo also to help with package creation and building.

@asbjornu
Copy link
Member

@emgarten Brilliant, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants