search

AngleSharp / AngleSharp.Js

:angel: Extends AngleSharp with a .NET-based JavaScript engine.
https://anglesharp.github.io
MIT License
103 stars 22 forks source link

Update packages to latest AngleSharp #37

Closed georgiosd closed 7 years ago

georgiosd commented 7 years ago

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
FlorianRappl commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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

FlorianRappl commented 7 years ago

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

georgiosd commented 7 years ago

Thank you :)