Skip to content

Update packages to latest AngleSharp#37

Merged
FlorianRappl merged 4 commits intoAngleSharp:masterfrom
georgiosd:master
May 7, 2017
Merged

Update packages to latest AngleSharp#37
FlorianRappl merged 4 commits intoAngleSharp:masterfrom
georgiosd:master

Conversation

@georgiosd
Copy link
Contributor

The tests seem to pass fine

========================================
Run-Unit-Tests
========================================
Executing task: Run-Unit-Tests
NUnit Console Runner 3.0.5813
Copyright (C) 2015 Charlie Poole

Runtime Environment
   OS Version: Microsoft Windows NT 10.0.15063.0
  CLR Version: 4.0.30319.42000

Test Files
    C:/Users/Georgios/Documents/GitHub/AngleSharp.Scripting/src/AngleSharp.Scripting.CSharp.Tests/bin/Release/AngleSharp.Scripting.CSharp.Tests.dll
    C:/Users/Georgios/Documents/GitHub/AngleSharp.Scripting/src/AngleSharp.Scripting.JavaScript.Generator.Tests/bin/Release/AngleSharp.Scripting.JavaScript.Generator.Tests.dll
    C:/Users/Georgios/Documents/GitHub/AngleSharp.Scripting/src/AngleSharp.Scripting.JavaScript.Tests/bin/Release/AngleSharp.Scripting.JavaScript.Tests.dll


Run Settings
    WorkDirectory: C:/Users/Georgios/Documents/GitHub/AngleSharp.Scripting/bin/0.5.0
    RuntimeFramework: net-4.0
    NumberOfTestWorkers: 12

Test Run Summary
    Overall result: Passed
   Tests run: 95, Passed: 95, Errors: 0, Failures: 0, Inconclusive: 0
     Not run: 0, Invalid: 0, Ignored: 0, Explicit: 0, Skipped: 0
  Start time: 2017-05-05 12:57:35Z
    End time: 2017-05-05 12:58:03Z
    Duration: 28.188 seconds

static Object Clr(this JsValue arg)
{
return arg.HasValue ? arg.Value.ToObject() : null;
return arg;
Copy link
Contributor

Choose a reason for hiding this comment

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

Reasons for this? The Clr function converts the JsValue (wrapping a non-JS value) to its original (non-JS) value.

@FlorianRappl
Copy link
Contributor

Looks good so far.

Just some notes:

  1. I can't accept this at the end as you PR to the master branch. PRs are only accepted to the devel branch. The master branch only contains the latest released version.
  2. Why have you changed the Clr function implementation? Reasoning? Side-effects?

Regarding 1. you would need to re-PR in any case (I will close it once you re-opened).

@georgiosd
Copy link
Contributor Author

Hey @FlorianRappl - the Clr function implementation change was necessary because it's not a nullable type any longer and the code wouldn't compile. Simples :)

I'll send you a PR on the devel branch

@georgiosd
Copy link
Contributor Author

Well, on the devel branch it seems you are referencing AngleSharp 0.10.0 already which doesn't exist, so I'm not sure if it makes sense to send you a PR there, especially if you're not going to publish a new nuget for the Scripting 0.5.x

@FlorianRappl
Copy link
Contributor

We could do a 0.5.1 (and make an exception to use the master branch, as v0.10 of AngleSharp is delayed). But for this to happen we need to solve the issue regarding the Clr function. It's not a nullable - fair enough. But why remove the logic? It should then return arg.Value.ToObject(), right? Or would Value?.ToObject() make more sense?

@georgiosd
Copy link
Contributor Author

@FlorianRappl you're 100% correct - not sure how I missed that. Thanks :)

@FlorianRappl FlorianRappl merged commit 77c6a48 into AngleSharp:master May 7, 2017
@FlorianRappl
Copy link
Contributor

FlorianRappl commented May 7, 2017

Looks good - I'll release a v0.5.1 then on NuGet!
Thanks @georgiosd 🍺 !

@georgiosd
Copy link
Contributor Author

Thank you :)

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.

2 participants