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

Feature: Add MAUI support #691

Merged
merged 46 commits into from
Dec 9, 2023
Merged

Conversation

OmnipotentOwl
Copy link
Contributor

@OmnipotentOwl OmnipotentOwl commented Nov 7, 2022

What kind of change does this PR introduce?
Add support for Dotnet MAUI to Sextant by introducing a new library package that follows the existing prior art of the Xamarin Forms package. Updates for the CI/CD Pipeline were made using references from the main ReactiveUI pipelines.

What is the current behavior?
There is no support for Dotnet MAUI in Sextant nor is there a sample provided for using Sextant with Dotnet MAUI applications

What is the new behavior?
Ability to use Sextant for ViewModel-driven navigation with ReactiveUI in Dotnet MAUI applications.

What might this PR break?
The introduction of the new library package shouldn't break the existing Xamarin Forms or Avalonia packages. However, the changes to support Dotnet 6/7/8 may introduce breaking changes in packages outside the scope of this new functionality.

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Other information:

  • Tested on Windows
  • Tested on iOS
  • Tested on Android Simulator
  • Tested on Android physical device
  • Tested on macOS Device

Fix name of initialize to reflect maui
Fix name of initialize to reflect maui
@dnfadmin
Copy link

dnfadmin commented Nov 7, 2022

CLA assistant check
All CLA requirements met.

@DanielCauser
Copy link

Hello, I cannot find a discussion regarding Sextant support for .net Maui. Is there any timeline for this?

@OmnipotentOwl
Copy link
Contributor Author

Hello, I cannot find a discussion regarding Sextant support for .net Maui. Is there any timeline for this?

Apologies for not checking this more often... This particular contribution has been something I have been working on slowly on the side and is not something I've discussed with the maintainers. I have the package in a local version for net6.0 and am looking at what it would take to move it to net8.0 hopefully without breaking some of the other libraries that are in the repo.

@ChrisPulman
Copy link
Member

I am fairly certain that this release will be a breaking change release anyway, so better to put people on a new course that is forward compatible.

@ChrisPulman
Copy link
Member

The tests are failing now, but at least the main build is good. Step by step it's getting there. Thank you for your efforts

@OmnipotentOwl
Copy link
Contributor Author

The tests are failing now, but at least the main build is good. Step by step it's getting there. Thank you for your efforts

For the API Approved tests, what do I need to do to pass them, or because of the new frameworks they are expected to fail?

@ChrisPulman
Copy link
Member

Could you please run the tests on your machine to get the API checks updated

@ChrisPulman
Copy link
Member

The tests are failing now, but at least the main build is good. Step by step it's getting there. Thank you for your efforts

For the API Approved tests, what do I need to do to pass them, or because of the new frameworks they are expected to fail?

When you run the tests two files are created, the verified file must match the discovered file, VS Code can be used to do the comparison

@OmnipotentOwl
Copy link
Contributor Author

The tests are failing now, but at least the main build is good. Step by step it's getting there. Thank you for your efforts

For the API Approved tests, what do I need to do to pass them, or because of the new frameworks they are expected to fail?

When you run the tests two files are created, the verified file must match the discovered file, VS Code can be used to do the comparison

When I run the tests locally I see the Approved files are generated as empty files but the tests still fail based on them being empty. Looking at the verified files they are also empty is this the expected output?

@OmnipotentOwl
Copy link
Contributor Author

One other strange observation is the net8.0 target framework name isnt pulling through so the approved version is named "ApiApprovalTests.Sextant..approved.txt"

@ChrisPulman
Copy link
Member

One other strange observation is the net8.0 target framework name isnt pulling through so the approved version is named "ApiApprovalTests.Sextant..approved.txt"

I can take a look and fix it if required, I have had to fix it in other repositories too so not a surprise this is failing

@OmnipotentOwl
Copy link
Contributor Author

I figured out the process for the updates to the txt files so they should be in the last commit now

@ChrisPulman
Copy link
Member

I will try to see what is happening at some point before my flight.

@ChrisPulman
Copy link
Member

if you can pull across the changes I made in #725 then this should fix the API issues, and mostly update the packages in the solution. I can't commit to your fork so I did this to try and help.

@OmnipotentOwl
Copy link
Contributor Author

if you can pull across the changes I made in #725 then this should fix the API issues, and mostly update the packages in the solution. I can't commit to your fork so I did this to try and help.

Thank you for that help, I cherry-picked your commit from that PR over to this one so we should be able to run the tests again.

Copy link

codecov bot commented Dec 9, 2023

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (3d141f4) 70.11% compared to head (67f8463) 62.13%.
Report is 3 commits behind head on main.

Files Patch % Lines
src/Sextant/Platforms/net/SextantExtensions.cs 50.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #691      +/-   ##
==========================================
- Coverage   70.11%   62.13%   -7.99%     
==========================================
  Files          32       27       -5     
  Lines         880      676     -204     
  Branches        0       82      +82     
==========================================
- Hits          617      420     -197     
+ Misses        263      226      -37     
- Partials        0       30      +30     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@OmnipotentOwl
Copy link
Contributor Author

Ok, now I think that's everything fixed for build and samples... Tests worked and passed and with the samples updated, we should be good. I do still want to run a manual test on my end for macos to make sure that works properly if I can get my build host working properly for that.

@ChrisPulman
Copy link
Member

I will be going to the airport in about 5 hours, then I will be offline for about 14 hours. Glenn may be able to assist you with pushing the build runs, but I am not sure of his movements.

@OmnipotentOwl OmnipotentOwl marked this pull request as ready for review December 9, 2023 17:19
@OmnipotentOwl
Copy link
Contributor Author

I will be going to the airport in about 5 hours, then I will be offline for about 14 hours. Glenn may be able to assist you with pushing the build runs, but I am not sure of his movements.

Thank you both for your support in this effort. I am hopeful that there will be no more issues with the samples and that we can wrap this up soon. I tested the code on MacOS and it ran fine without any issues that I could visibly see with the sample app.

@OmnipotentOwl
Copy link
Contributor Author

Are there any more criteria you would like to see added before merging these changes? If no do you want me to merge the changes or is there a preview build you all would put out with this to field test it any further?

@glennawatson glennawatson merged commit a1ba7af into reactiveui:main Dec 9, 2023
3 of 5 checks passed
@OmnipotentOwl OmnipotentOwl deleted the feature/maui branch December 11, 2023 16:22
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants