search

keenlabs / keen-sdk-net

A .NET SDK for the Keen IO API
MIT License
37 stars 24 forks source link

[WIP] Port Event cache portable #105

Closed Donistivanov closed 6 years ago

Donistivanov commented 6 years ago

Works at least to the extent of known race conditions

Is based off https://github.com/keenlabs/keen-sdk-net/pull/103

look to close #65

Donistivanov commented 6 years ago

What is the proper way to debug this failed build?

masojus commented 6 years ago

@Donistivanov Normally you would just look at the logs linked to by the "Details" link, but I'm not sure if you have permission to view those. Are you able to see what it spits out here? https://ci.appveyor.com/project/masojus/keen-sdk-net/build/1.0.37-dotnetsummer

Of course, if it's an issue with all the changes I've been making and the netstandard build and the dotnet build vs msbuild issues (which is likely) then it's up to your DevOps guy (me) to fix it. I would only worry if you see actual compilation errors referring to something to do with the EventCachePortable or other code you touched in this change. This APpVeyor setup is brand new and we're evolving it side by side with the addition of the NetStandard binaries, so some rough patches are expected :)

jongalloway commented 6 years ago

The appveyor build logs are public by default. Direct to this build log: https://ci.appveyor.com/project/masojus/keen-sdk-net/build/1.0.37-dotnetsummer

Looks like the problem is PCLStorage: https://ci.appveyor.com/project/masojus/keen-sdk-net/build/1.0.37-dotnetsummer#L114

It doesn't look like PCLStorage currently supports .NET Standard: https://github.com/dsplaisted/PCLStorage/issues/53

Maybe this would work instead? https://www.nuget.org/packages/PCLStorage.Standard/

Donistivanov commented 6 years ago

Yes I can. It was weird that using the original PCL package made everything build on my computer. I will mark this as WIP for now since there may be more to this than I initially saw.

masojus commented 6 years ago

@jongalloway Yeah that's what I figured. It's what @baumatron had posited in Issue #65

masojus commented 6 years ago

@Donistivanov I would pick Immo's brain in Slack. This is likely due to the version of msbuild used by VS 2017 vs the dotnet build/dotnet msbuild commands is different. It could also be the version mismatch between your machine (VS 15.3 preview) and AppVeyor's VS 2017 version (15.1)

masojus commented 6 years ago

The real question is should PCLStorage build in a netstandard project, and my expectation was that, no, it wouldn't due to portability analysis reports, but I was remaining optimistic :)

masojus commented 6 years ago

I'm holding off on merging this one until we get a better idea of what is expected with PCLStorage vs PCLStorage.Standard and get this working as expected with the VS 15.3 tools. @Donistivanov feel free to update from master and push a new change to see if the recent CI config changes make any difference. If things are still looking broken, then let's try out the PCLStorage.Standard nuget package, and whatever changes that implies to adapt to its interfaces, as long as they don't seem extensive. If this starts looking to be a headache, considering the existing bugs and potential re-write of this class, I'd shelve it for now.

Donistivanov commented 6 years ago

I am closing this pull request in favor of the work at https://github.com/keenlabs/keen-sdk-net/pull/121