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

Add EFCore test project #123

Merged
merged 2 commits into from
May 26, 2017
Merged

Add EFCore test project #123

merged 2 commits into from
May 26, 2017

Conversation

AlekseyMartynov
Copy link
Contributor

Okay, @kaatula this time we do it right

@AlekseyMartynov AlekseyMartynov self-assigned this May 25, 2017
@AlekseyMartynov AlekseyMartynov requested a review from kaatula May 25, 2017 14:50
Copy link

@kaatula kaatula left a comment

Choose a reason for hiding this comment

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

Approved with minor questions

@@ -28,6 +28,13 @@ test_script:
-output:coverage_netcore.xml

- >-
OpenCover\tools\OpenCover.Console -returntargetcode -register:user -oldstyle
Copy link

Choose a reason for hiding this comment

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

That is a brand new appveyor, how do you the oldstyle??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link

Choose a reason for hiding this comment

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

🙈


namespace DevExtreme.AspNet.Data.Tests.EFCore {

class SampleLoadOptions : DataSourceLoadOptionsBase {
Copy link

Choose a reason for hiding this comment

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

Not sure I got it...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

DataSourceLoadOptionsBase is abstract.

Copy link

@kaatula kaatula May 26, 2017

Choose a reason for hiding this comment

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

Yes, but why it is abstract?
Just to have a excuse to create such strange-looking classes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤡

@@ -4,6 +4,8 @@

#if DEBUG
[assembly: InternalsVisibleTo("DevExtreme.AspNet.Data.Tests")]
[assembly: InternalsVisibleTo("DevExtreme.AspNet.Data.Tests.EF6")]
Copy link

Choose a reason for hiding this comment

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

InternalsVisibleTo looks like a bad sign, doesn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, not at all.

Copy link

Choose a reason for hiding this comment

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

Thank you, I downvoted :(

@AlekseyMartynov AlekseyMartynov merged commit 8bcbb97 into DevExpress:master May 26, 2017
@AlekseyMartynov AlekseyMartynov deleted the efcore-tests branch May 26, 2017 12:59
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.

2 participants