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

PR for fundamental core structure changes #51

Merged
merged 19 commits into from
Mar 14, 2019
Merged

PR for fundamental core structure changes #51

merged 19 commits into from
Mar 14, 2019

Conversation

jamesmcroft
Copy link
Member

Describe the changes made
This PR contains fundamental changes to the MADE App Components libraries which are described below.

Changes to minimum target version

Changes have been made to target the lowest minimum target version of .NET Standard and UWP so that the libraries can be installed with more projects.

This change sees the .NET Standard version lower from 2.0 to 1.4 and the UWP version to cover all versions instead of Fall Creators Update+

Changes to incorporate XPlat

Other changes associated with this PR include adding support for XPlat where MADE was using custom built alternatives. These changes have been kept to a minimum but as a co-owner of the project, I suggest that we start incorporating XPlat as the base API layer for building controls and other functions in MADE.

Some examples include:

  • MADE has a helper class for UI action dispatching which uses native APIs. XPlat contains a CoreDispatcher which does the same thing so has been replaced with it for consistency.

  • MADE has a color, thickness and orientation class. XPlat contains alternatives which are cross-platform and have been replaced where used.

Changes to NuGet package references

All package references have been updated to the latest versions.

The UWP libraries have been downgraded to a supported version (6.1.9).

Included as part of this change has been to replace the older PCL variant of MvvmLight with the .NET Standard version. This will have a breaking change on users who are currently using this as the CommonServiceLocator is no longer used which is described here: http://www.mvvmlight.net/std10

There currently is an issue outstanding in the MvvmLight project that users who are implementing Android functionality and use the MvvmLight android support extension cannot use this. There is an active PR for this (lbugnion/mvvmlight#25) but has been open since March last year.

There is also an open PR from myself with MvvmLight regarding Android support packages references (lbugnion/mvvmlight#60) with a PR (lbugnion/mvvmlight#61) which have not yet been resolved.

@tom-made would it be wise if we forked MvvmLight and created our own internal packages to resolve these issues?

What issues are associated?
No associated issues

@jamesmcroft jamesmcroft requested review from Tarc94 and tom-made and removed request for Tarc94 March 14, 2019 11:11
@jamesmcroft jamesmcroft added this to the v1.0 milestone Mar 14, 2019
@jamesmcroft jamesmcroft requested a review from Tarc94 March 14, 2019 12:17
Tarc94
Tarc94 previously approved these changes Mar 14, 2019
Copy link

@Tarc94 Tarc94 left a comment

Choose a reason for hiding this comment

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

This looks good to me, can't see any issues.

Copy link
Contributor

@tom-made tom-made left a comment

Choose a reason for hiding this comment

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

Some changes need to be made here and added some questions.

Copy link
Contributor

@tom-made tom-made left a comment

Choose a reason for hiding this comment

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

Good for these to be merged

@jamesmcroft jamesmcroft merged commit c14cb66 into master Mar 14, 2019
@jamesmcroft jamesmcroft deleted the cleanup branch March 14, 2019 14:40
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.

3 participants