search

YUICompressor-NET / YUICompressor.NET

Port of Yahoo!'s Java YUICompressor to .NET
BSD 3-Clause "New" or "Revised" License
122 stars 61 forks source link

Updated projects to use .Net452 #20

Closed solidcloudio closed 7 years ago

solidcloudio commented 7 years ago

Existing version failed as Windows Update removed .net 2.0 Framework. Updated projects to use .net 4.5.2

PureKrome commented 7 years ago

Tests failing, btw.

solidcloudio commented 7 years ago

Tests were failing because of: 

    public int LineNumberOfTaskNode
    {
        get { throw new NotImplementedException(); }
    }

    public int ColumnNumberOfTaskNode
    {
        get { throw new NotImplementedException(); }
    }

Not sure why they didn't fail before.. This is just in the Stub.. updated to 

    public int LineNumberOfTaskNode
    {
        get { return 0; }
    }
    public int ColumnNumberOfTaskNode
    {
        get { return 0; }
    }

Tests Pass..

Taritsyn commented 7 years ago

It would be wise to downgrade .NET Framework version from 4.5.2 to 4.5.1. Key libraries are still targeted on .NET Framework 4.5.1 (e.g., Microsoft.AspNetCore.Mvc and Microsoft.EntityFrameworkCore).

freeranger commented 7 years ago

I don't think we should accept this PR - nobody else has complained it is not working as is.

solidcloudio commented 7 years ago

Whats the point? why leave "throw new NotImplementedException(); " In place?

Isn't it obvious the tests will fail? Are they being run?

image

freeranger commented 7 years ago

If all the PR did was fix that issue then that's one thing but it upgrades the .NET Framework version too. Even if this was something desirable, it should be in a separate PR to one for fixing a bug.

solidcloudio commented 7 years ago

touche...

PureKrome commented 7 years ago

I'll do this.

https://github.com/PureKrome/YUICompressor.NET/issues/22 created.

freeranger commented 7 years ago

BTW, All the tests pass with the current code: image

Perhaps upgrading to 4.x causes some extra method calls in the stub which is why they failed for @solidcloudio when he upgraded it.