Skip to content

NH-3905 - Implement Async operations #677

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

Merged
merged 12 commits into from
Sep 1, 2017

Conversation

fredericDelaporte
Copy link
Member

@fredericDelaporte fredericDelaporte commented Aug 29, 2017

NH-3905 This is a rebase of #588.

For eliminating the different directions async future have taken before (and for easing the rebase), most future related changes have been squashed into first @maca88 commit.
For allowing to generates async files and compile most commits individually, most async generation setup commits have been moved prior to first maca88 commit. This means most upgrade async generator commits are squashed and the generator version is 0.3.6 from almost "the beginning" of this rebase.
Some additional squashing have been made (especially changes reverted some commits later).
Most "regen async" commits have been regenerated with 0.3.6 async generator version.

@hazzik
Copy link
Member

hazzik commented Aug 29, 2017

@fredericDelaporte I think we need to move "Regenerate Async" to the latest commit. Made in one step.

@hazzik
Copy link
Member

hazzik commented Aug 29, 2017

I did further combination of commits and moved async generation to the top.

hazzik
hazzik previously approved these changes Aug 30, 2017
@fredericDelaporte
Copy link
Member Author

I think we need to move "Regenerate Async" to the latest commit. Made in one step.

Thought the same at first but this means previous commits cannot compile without first generating async files, modifying the tree. It may not be needed to be able to compile previous commits but I still rather avoid letting many non compilable commits in history. Maybe just one regen right after remaining maca88 commit would have been enough for this purpose though.

@hazzik
Copy link
Member

hazzik commented Aug 30, 2017 via email

@fredericDelaporte
Copy link
Member Author

fredericDelaporte commented Aug 30, 2017

Moved most prep prior to async generation commit, done some additional squashing, dropped syntax workaround for async gen no more needed, cleaned some undue added namespace, split maca88 commit between prep and what is requiring to already have async files generating (Linq & Future).

This yield an intial async generation, followed by changes for exploiting it for linq & futures, followed by a regen.

All commits compile separately.

Some of your dropped partial on the two test files under Async folder are back: I do not know why there were removed in your rebase, because when I generate async on your rebase, it does add them back. Anyway, undue partial in some generated code is not a concern I think.

@hazzik
Copy link
Member

hazzik commented Aug 30, 2017

All good. I think we might need to move Future refactoring into the future ticket (we had one for 5.0). Or at least mention this refactoring at the ticket.

hazzik
hazzik previously approved these changes Aug 30, 2017
@hazzik hazzik changed the title Async generator NH-3905 - Support for Async operations Aug 30, 2017
@hazzik hazzik changed the title NH-3905 - Support for Async operations NH-3905 - Implement Async operations Aug 30, 2017
Copy link
Member Author

@fredericDelaporte fredericDelaporte left a comment

Choose a reason for hiding this comment

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

I think we might need to move Future refactoring into the future ticket (we had one for 5.0). Or at least mention this refactoring at the ticket.

Going to mention that in NH-3905. This refactoring (IFutureEnumerable instead of IEnumerable) was really needed for async case, a bit less without it. And the previous refactoring (NH-4030) was mainly about Linq Future implementation, not its overall implementation.

/// <summary>
/// Get an executable instance of <c>IQueryOver&lt;TRoot&gt;</c>,
/// to actually run the query.</summary>
public IQueryOver<TRoot,TRoot> GetExecutableQueryOver(ISession session)
Copy link
Member Author

Choose a reason for hiding this comment

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

Whitespace miss probably introduced during rebasing. I am anyway rebasing again, while rewording commits for including Jira number.

@hazzik
Copy link
Member

hazzik commented Sep 1, 2017

I'm going to merge as soon as tests pass. Any objections?

@fredericDelaporte
Copy link
Member Author

Ok for me, going to do that myself in a couple of minutes if you do not seem online.

@hazzik hazzik merged commit 4d99b2b into nhibernate:master Sep 1, 2017
@fredericDelaporte fredericDelaporte deleted the AsyncGenerator branch September 1, 2017 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants