Skip to content

Query: API Cleanup #20767

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 1 commit into from
Apr 28, 2020
Merged

Query: API Cleanup #20767

merged 1 commit into from
Apr 28, 2020

Conversation

smitpatel
Copy link
Contributor

No description provided.

@smitpatel
Copy link
Contributor Author

@dotnet/efcore - This PR makes some API breaks & changes nullability annotations which could break functionality where API was incorrectly used. All the changes are due to incorrect API in first place. If things are done right way, then it all works. Please review, if needed, we can bring this to design meeting on Wednesday.

roji
roji previously requested changes Apr 28, 2020
View reviewed changes
Copy link
Member

@roji roji left a comment •
edited
Loading

Choose a reason for hiding this comment

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

See only note about niladic management in SqlFunctionExpression.

I think nullability annotation changes are fine, as we said in triage...

@smitpatel smitpatel force-pushed the smit/relationaldocs branch from e500f5d to 656943a Compare April 28, 2020 10:30
@smitpatel smitpatel requested review from AndriySvyryd and roji April 28, 2020 10:40
@smitpatel
Copy link
Contributor Author

@AndriySvyryd - Added back obsolete methods.

@smitpatel smitpatel dismissed roji’s stale review April 28, 2020 10:44

Distinction is intentional. I posted alternatives if distinction is not preferred. The dislike towards distinction has nothing to do with ctor/factory methods. Please file a new issue with exact problem you are seeing.

@bricelam bricelam mentioned this pull request Apr 28, 2020
Closed
95 tasks
@smitpatel smitpatel force-pushed the smit/relationaldocs branch from 656943a to 62a5f50 Compare April 28, 2020 12:45
@smitpatel
Copy link
Contributor Author

@dotnet/aspnet-build @bricelam - What is causing incorrect names in test reporting in azure pipeline?

image

@ajcvickers
Copy link
Contributor

With regard to obsolete methods, we should err on the side of doing them. Avoiding simple binary breaks has value--it just takes one being used by some provider/extension to prevent that provider/extension being usable without a rebuild.

@smitpatel
Copy link
Contributor Author

Are we testing that 3.1 providers work with 5.0 EF Core? If there is going to be one API which causes break then there is no benefit of going through trouble of having 99 other API avoid break. It won't work either way.

@ajcvickers
Copy link
Contributor

@smitpatel Yes, we are going to do more testing with things that currently work in 3.1.

@dougbu
Copy link
Contributor

dougbu commented Apr 28, 2020

@dotnet/aspnet-build @bricelam - What is causing incorrect names in test reporting in azure pipeline?

@anurse could this be related to your recent Arcade update?

@smitpatel smitpatel merged commit c11deea into master Apr 28, 2020
@smitpatel smitpatel deleted the smit/relationaldocs branch April 28, 2020 21:08
@ajcvickers ajcvickers mentioned this pull request Apr 30, 2020
Closed
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@AndriySvyryd AndriySvyryd AndriySvyryd approved these changes

@maumar maumar Awaiting requested review from maumar

@roji roji Awaiting requested review from roji

Assignees
No one assigned
Labels
providers-beware
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@smitpatel @ajcvickers @dougbu @roji @AndriySvyryd