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

Addressing API Review feedback for temporal tables feature: #25430

Merged
merged 1 commit into from
Aug 6, 2021

Conversation

maumar
Copy link
Contributor

@maumar maumar commented Aug 5, 2021

  • converted QueryRootCreator into general purpose helper service,
  • renamed WithHistoryTable to UseHistoryTable,
  • removed Temporal from entity type extensions,
  • added dependency objects to temporal convention.

@maumar maumar requested a review from a team August 5, 2021 01:15
@@ -6,21 +6,21 @@
namespace Microsoft.EntityFrameworkCore.Query
{
/// <inheritdoc/>
public class QueryRootCreator : IQueryRootCreator
public class NavigationExpansionExtensibilityHelper : INavigationExpansionExtensibilityHelper
Copy link
Contributor Author

Choose a reason for hiding this comment

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

NavigationExpansionExtensibilityService? (or something else?)

Copy link
Member

Choose a reason for hiding this comment

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

INavigationExpansionHandler?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

wouldn't handler imply too much? this thing only aids the nav expansion in slight ways. the nav expansion proper is "handled" by the nav expanding expression visitor

- converted QueryRootCreator into general purpose helper service,
- renamed WithHistoryTable to UseHistoryTable,
- removed Temporal from entity type extensions,
- added dependency objects to temporal convention.
@maumar maumar merged commit ccd7d07 into main Aug 6, 2021
@maumar maumar deleted the maumar_api_review branch August 6, 2021 22:00
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