-
Notifications
You must be signed in to change notification settings - Fork 934
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
Conversation
@fredericDelaporte I think we need to move "Regenerate Async" to the latest commit. Made in one step. |
I did further combination of commits and moved async generation to the top. |
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. |
Then we, probably, need to clash all these prep commits into one.
|
66d7ae6
to
725a192
Compare
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 |
All good. I think we might need to move |
There was a problem hiding this 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<TRoot></c>, | ||
/// to actually run the query.</summary> | ||
public IQueryOver<TRoot,TRoot> GetExecutableQueryOver(ISession session) |
There was a problem hiding this comment.
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.
725a192
to
d18aaed
Compare
… async code generation
I'm going to merge as soon as tests pass. Any objections? |
Ok for me, going to do that myself in a couple of minutes if you do not seem online. |
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.