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

Resource loading #815

Merged
merged 6 commits into from Mar 16, 2017

Conversation

Projects
None yet
4 participants
@StephaneDelcroix
Member

StephaneDelcroix commented Mar 14, 2017

Description of Change

More generic resourceLoading mechanism for the Previewer, so the same API can be reused to load different kind of resources.

Bugs Fixed

/

API Changes

/

Behavioral Changes

/

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard
  • Consolidate commits as makes sense

StephaneDelcroix added some commits Mar 13, 2017

@StephaneDelcroix StephaneDelcroix requested a review from alanmcgovern Mar 14, 2017

@@ -195,6 +195,8 @@ public override bool Execute(out IList<Exception> thrownExceptions)
Logger.LogLine(2, "done");
}
Logger.LogLine(2, "");

This comment has been minimized.

@alanmcgovern

alanmcgovern Mar 14, 2017

Contributor

Is this a debugging helper?

@alanmcgovern

alanmcgovern Mar 14, 2017

Contributor

Is this a debugging helper?

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Mar 14, 2017

Member

no, it's not. adding that white line makes the build output clearer

@StephaneDelcroix

StephaneDelcroix Mar 14, 2017

Member

no, it's not. adding that white line makes the build output clearer

Show outdated Hide outdated Xamarin.Forms.Xaml/XamlLoader.cs
var xaml = Internals.XamlLoader.XamlFileProvider?.Invoke(type);
#pragma warning restore 0618
if (xaml != null && ResourceLoader.ResourceProvider != null)
return xaml;

This comment has been minimized.

@alanmcgovern

alanmcgovern Mar 14, 2017

Contributor

Why are you checking that ResourceProvider != null here? I presume the idea is that we will either be using XamlFileProvider or we'll be using ResourceLoader.ResourceProvider which means if xaml is non-null then ResourceProvider will always be null.

@alanmcgovern

alanmcgovern Mar 14, 2017

Contributor

Why are you checking that ResourceProvider != null here? I presume the idea is that we will either be using XamlFileProvider or we'll be using ResourceLoader.ResourceProvider which means if xaml is non-null then ResourceProvider will always be null.

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Mar 14, 2017

Member

this ResourceProvider != null check is not really required, it's only make sure that if you were foolish enough to set both, the ResourceProvider will prevail.

@StephaneDelcroix

StephaneDelcroix Mar 14, 2017

Member

this ResourceProvider != null check is not really required, it's only make sure that if you were foolish enough to set both, the ResourceProvider will prevail.

This comment has been minimized.

@alanmcgovern

alanmcgovern Mar 14, 2017

Contributor

I think you've just hit the nail on the head - if you set both, and XamlFileProvider does give valid data, then ResourceProvider will not prevail. Was that expected?

@alanmcgovern

alanmcgovern Mar 14, 2017

Contributor

I think you've just hit the nail on the head - if you set both, and XamlFileProvider does give valid data, then ResourceProvider will not prevail. Was that expected?

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Mar 14, 2017

Member

🤦‍♂️

@StephaneDelcroix

StephaneDelcroix Mar 14, 2017

Member

🤦‍♂️

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Mar 14, 2017

Member

this is a case when I should rebase to hide my embarrassment

@StephaneDelcroix

StephaneDelcroix Mar 14, 2017

Member

this is a case when I should rebase to hide my embarrassment

@samhouts samhouts merged commit 467c1be into master Mar 16, 2017

1 check passed

Windows-Debug-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Stable - Cycle 9 :: Windows Debug : Tests passed: 3744, ignored: 10
Details

@samhouts samhouts deleted the resourceLoading branch Mar 16, 2017

@samhouts samhouts added D-15.4 and removed cla-not-required labels Oct 10, 2017

@samhouts samhouts modified the milestones: 2.3.0, 2.3.5 Jun 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment