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.

Sign up for GitHub

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

Jump to bottom

Adds Tizen backend #2360

Merged
merged 254 commits into from
Apr 15, 2022
Merged

Adds Tizen backend #2360

merged 254 commits into from
Apr 15, 2022

Conversation

rookiejava
Copy link
Contributor

Description of Change

This PR adds Tizen backend.

Demo

- Weather `21
  • TV

- Mobile

Controls Sample

Special thanks to .NET MAUI folks (Yes, you!) for always helping us behind the scenes.

@rmarinho
Copy link
Member

rmarinho commented Sep 2, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@rookiejava rookiejava force-pushed the add-tizen-handler branch 2 times, most recently from 95e73a5 to a8ddfab Compare September 7, 2021 09:36
mattleibow
mattleibow reviewed Sep 8, 2021
View reviewed changes
Copy link
Member

@mattleibow mattleibow left a comment

Choose a reason for hiding this comment

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

This is a big PR! Only about 1/3 of the way, and already looking really good.

I have a few questions so far, but more not sure of the differences I spotted to what we did. Probably good reasons.

src/Compatibility/Core/src/Tizen/Extensions/BrushExtensions.cs Outdated Show resolved Hide resolved
src/Core/src/Platform/MauiContext.Tizen.cs Outdated Show resolved Hide resolved
src/Compatibility/Core/src/Tizen/Renderers/ActivityIndicatorRenderer.cs Outdated Show resolved Hide resolved
src/Essentials/samples/Samples/Startup.cs Outdated Show resolved Hide resolved
src/Core/src/ImageSources/IImageSourceService.cs Outdated Show resolved Hide resolved
src/Core/src/Graphics/PaintExtensions.Tizen.cs Outdated Show resolved Hide resolved
src/Essentials/samples/Samples/Startup.cs Outdated Show resolved Hide resolved
src/Core/src/Platform/Tizen/SkiaGraphicsView.Tizen.cs Outdated Show resolved Hide resolved
@mattleibow
Copy link
Member

Were you able to get the DeviceTests projects working? I cheated a bit on Windows and excluded all the handlers except one for now - you could do the same. This way you can write tests for handlers/concepts that might be missed.

We might not have a way to run on CI on every commit just yet, but could be a way for you and us to just run and see if we broke anything before we release.

https://github.com/dotnet/maui/blob/main/src/Core/tests/DeviceTests/Core.DeviceTests.csproj#L66

@rookiejava
Copy link
Contributor Author

Thank you @mattleibow for reviewing. ❤️

Were you able to get the DeviceTests projects working?

Ok. Let me check we can get the DeviceTest project.

@rmarinho
Copy link
Member

rmarinho commented Sep 9, 2021

/azp run

@azure-pipelines
Copy link

Pull request contains merge conflicts.

@rookiejava rookiejava force-pushed the add-tizen-handler branch 3 times, most recently from f7b6200 to 1892c8a Compare September 30, 2021 08:57
@rookiejava rookiejava force-pushed the add-tizen-handler branch 2 times, most recently from d84c486 to 03cc6e4 Compare October 12, 2021 06:43
@Eilon Eilon added the area-setup Installation, setup, requirements, maui-check, workloads, platform support label Oct 18, 2021
@rookiejava rookiejava force-pushed the add-tizen-handler branch 2 times, most recently from 09f22ee to 9954658 Compare October 25, 2021 04:17
@MartyIX
Copy link
Contributor

MartyIX commented Nov 5, 2021

@rookiejava Will Tizen be supported by Microsoft if/when this PR is merged?

@rookiejava
Copy link
Contributor Author

@rookiejava Will Tizen be supported by Microsoft if/when this PR is merged?

Yes, I think/hope so. :-)

@rookiejava rookiejava force-pushed the add-tizen-handler branch 3 times, most recently from 1868bdf to 3b60dd0 Compare November 23, 2021 00:45
@rookiejava rookiejava force-pushed the add-tizen-handler branch 2 times, most recently from e962b53 to f044413 Compare December 7, 2021 09:51
@mattleibow
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@mattleibow
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@mattleibow
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@mattleibow mattleibow dismissed jonathanpeppers’s stale review April 15, 2022 07:03

All the things have been thinged!

mattleibow
mattleibow approved these changes Apr 15, 2022
View reviewed changes
Directory.Build.props
Comment on lines +18 to +20
<IncludeTizenTargetFrameworks Condition="'$(CI)' == 'true' or '$(TF_BUILD)' == 'true' or
Exists('$(DOTNET_ROOT)\sdk-manifests\$(DotNetVersionBand)\samsung.net.sdk.tizen\WorkloadManifest.json') or
Exists('$(ProgramFiles)\dotnet\sdk-manifests\$(DotNetVersionBand)\samsung.net.sdk.tizen\WorkloadManifest.json')">true</IncludeTizenTargetFrameworks>
Copy link
Member

Choose a reason for hiding this comment

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

This condition needs love, but that is for another PR. I see we had to also add TF_BUILD here because CI is declared in the Environment.props at the bottom of this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We plan to provide attrubute that check tizen workload is installed or not in next tizen workload release. So, MAUI just using this here.
/cc @JoonghyunCho will do. right? :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I will give some love to this condition by replacing it to property after updating Tizen workload. :)

eng/pipelines/common/provision.yml
Comment on lines +110 to +117
- pwsh: |
[xml] $fileXml = Get-Content "eng\Versions.props"
$DotNetVersionBand = $fileXml.SelectSingleNode("Project/PropertyGroup/DotNetVersionBand").InnerText
echo "Installing .NET $DotNetVersion"
Invoke-WebRequest 'https://raw.githubusercontent.com/Samsung/Tizen.NET/main/workload/scripts/workload-install.ps1' -OutFile 'workload-install.ps1'
.\workload-install.ps1 -t $DotNetVersion
displayName: 'Install Tizen'

Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is needed anyore.

@mattleibow mattleibow enabled auto-merge (squash) April 15, 2022 07:07
auto-merge was automatically disabled April 15, 2022 07:24

Head branch was pushed to by a user without write access

@mattleibow mattleibow enabled auto-merge (squash) April 15, 2022 07:24
@mattleibow
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@mattleibow mattleibow merged commit 438aef2 into dotnet:main Apr 15, 2022
@congzhangzh
Copy link

magic:)

@github-actions github-actions bot locked and limited conversation to collaborators Dec 25, 2023
@samhouts samhouts added the fixed-in-6.0.300-rc.2 Look for this fix in 6.0.300-rc.2! label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jonathanpeppers jonathanpeppers jonathanpeppers left review comments

@JoonghyunCho JoonghyunCho JoonghyunCho left review comments

@mattleibow mattleibow mattleibow approved these changes

@rmarinho rmarinho rmarinho approved these changes

@Eilon Eilon Awaiting requested review from Eilon

Assignees
No one assigned
Labels
area-setup Installation, setup, requirements, maui-check, workloads, platform support fixed-in-6.0.300-rc.2 Look for this fix in 6.0.300-rc.2!
Projects
None yet
Milestone
Backlog
Development

Successfully merging this pull request may close these issues.