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

Fixes #74 #76

Merged
merged 2 commits into from
Nov 9, 2017
Merged

Fixes #74 #76

merged 2 commits into from
Nov 9, 2017

Conversation

rasmuseeg
Copy link
Contributor

This is propperly not the right way to do it. But lets talk about it.

@rasmuseeg
Fixes #74
b4f7ec7 
This is propperly not the right way to do it. But lets talk about it.
@rasmuseeg
Fixed typo in description
fba9c1d
@mattbrailsford
Copy link
Collaborator

That's not necessarily a bad approach. My only questions are 1) do we think we would need the same check in the other "ThisToThat" methods? 2) Whether we should streamline the following GetTargetDataTypeDefinition call to reuse the retrieved property data type to save potentially loading that twice?

@rasmuseeg
Copy link
Contributor Author

I did some testing to see if it was necessary to do the same thing on the others "ThisToThat" methods.
But because, it has been done/set in DbToString, which writes to the umbraco cache, it shouldn't.

@mattbrailsford mattbrailsford merged commit 4c0f7a0 into umco:master Nov 9, 2017
@mattbrailsford mattbrailsford added this to the 1.6 milestone Nov 9, 2017
@rasmuseeg rasmuseeg deleted the patch-2 branch November 10, 2017 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
1.6
Development

Successfully merging this pull request may close these issues.
2 participants
@rasmuseeg @mattbrailsford