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

Multitarget Views.Desktop.Common #1067

Merged
merged 2 commits into from
Dec 19, 2019
Merged

Multitarget Views.Desktop.Common #1067

merged 2 commits into from
Dec 19, 2019

Conversation

TysonMN
Copy link
Contributor

@TysonMN TysonMN commented Dec 19, 2019

Description of Change

Recreating PR #1066, which was closed when I deleted by branch.

Changes SkiaSharp.Views.Desktop.Common to multitarget

  • .NET Standard 2.0,
  • .NET Framework 4.5, and
  • .NET Core 3.0.

Also removes SkiaSharp.Views.Desktop.Common.NetStandard and all references to it (which wasn't the case in PR #1066).

I don't know if any part of the automated builds/devops needs to change as well. Someone should check that.

Bugs Fixed

Related to issue #1029. This PR gets us closer to dual targeting .NET Framework and .NET Core for SkiaSharp.Views.WPF.

API Changes

None

Behavioral Changes

None (intended)

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • NA
  • Changes adhere to coding standard
  • Updated documentation

@TysonMN TysonMN mentioned this pull request Dec 19, 2019
4 tasks
@TysonMN
Copy link
Contributor Author

TysonMN commented Dec 19, 2019

Build still fails :(

Quoting #1066 (comment)

Still failing for some reason. I'm AFK right now, I'll have a look as soon as I get back. Thanks for the
PR. Not sure how I missed this one.

You might also want to have a look at the nuspec file and add the new platform.

There are no more references to SkiaSharp.Views.Desktop.Common.NetStandard (because I searched within all files in the repo and either removed the reference or changed it to SkiaSharp.Views.Desktop.Common). None of those references were in a nuspec file, so I didn't make any changes to those.

And you are welcome for the PR :) Feel free to delegate any work to me to get this to build.

@TysonMN
Copy link
Contributor Author

TysonMN commented Dec 19, 2019

I created PR #1068 to see if the first commit in the branch for this PR passes the automated build, which might help us figure out how to get both commits in the branch for this PR to pass the automated build.

@mattleibow
Copy link
Contributor

@bender2k14 The build is actually passing.

The reason for the red is that one of the samples is failing because a public PR does not have access to the signing keys required for iOS. I used to have a public CI but we don't seem to have a big enough Azure to use that - or something - so I have to just use the private builds for now. Trying to get more parallel builds in public so we get better status. Sorry about that.

You don't need to do #1068

@TysonMN
Copy link
Contributor Author

TysonMN commented Dec 19, 2019

The build is actually passing.

Oh, great :)

...Trying to get more parallel builds in public so we get better status. Sorry about that.

That's ok.

Is there work left to do for this PR then?

@mattleibow mattleibow changed the title Multitarget Views.Desktop.Common 2 Multitarget Views.Desktop.Common Dec 19, 2019
@mattleibow
Copy link
Contributor

Nothing :)

@mattleibow mattleibow merged commit 36693bd into mono:dev/vs-2019 Dec 19, 2019
@TysonMN TysonMN deleted the dev/vs-2019_1029_multitarget_Views.Desktop.Common branch December 19, 2019 17:14
@mattleibow mattleibow added this to the v1.68.2 milestone Apr 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants