NH-3303 - Refactoring and fixes for custom SQL code #360
Conversation
| var lockModes = new List<LockMode>(); | ||
| var resultColumnProcessors = new List<IResultColumnProcessor>(); | ||
| var resultTypes = new List<IType>(); | ||
| var transformerAliases = new List<string>(); |
There was a problem hiding this comment.
can we remove this unnecessary changes?
There was a problem hiding this comment.
I added the this. prefixes to get clearer distinction between local variable and instance fields, as in the original code the only difference between field and local variable names was the casing. I can revert that change to get a clearer view of the actual changes that this commit introduces, but it makes reading/understanding the code harder.
|
Reverted changes to local variable names and field initialisation as requested. |
|
@ggeurts For a while now we often follow Resharper's default settings, which is to warn for presence of Given the large size of the PR, this should not be taken as a request to change this right now. |
hazzik
changed the title
|
I am aware this is a deviation from the Hibernate code base, but given that my company uses a fair amount of custom sql code with NHiberate and have fixed a fair amount of related issues with it, a refactoring of the custom sql code base is desirable to keep the code maintainable. The changes and their rationale have been well documented in pull request #161. |
|
@hazzik I think it was partly a generic hesitance to diverge from Hibernate, and in particular I'm interested in Hibernate Loader refactoring done in 2012-2014 and how porting that might impact these changes (or the other way around). |
|
Can you please enable Allow edits from maintainers PR option? |
|
Replaced by #1945. |
An up-to-date version of changes proposed in pull request #161.