Fixes around namespaces and Fluent API calls #25435
Conversation
src/EFCore.Design/Scaffolding/Internal/CSharpDbContextGenerator.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Design/Scaffolding/Internal/CSharpDbContextGenerator.cs
Outdated
Show resolved
Hide resolved
roji
force-pushed
the
roji/CodeFragmentNamespaces
branch
from
d8ce5d5 to
ad51db0
Compare
Co-authored-by: Andriy Svyryd <AndriySvyryd@users.noreply.github.com>
| Check.NotNull(stringBuilder, nameof(stringBuilder)); | ||
|
|
||
| foreach (var annotation in annotations) | ||
| foreach (var annotation in annotations.OrderBy(a => a.Name)) |
There was a problem hiding this comment.
Why do you need to reorder?
There was a problem hiding this comment.
I removed this method which was dead code (#25435 (comment)), but the same ordering is applied to raw annotations in the new GenerateAnnotations. It's for alphabetical determinism etc., nothing too important.
There was a problem hiding this comment.
I wonder whether using SortedDictionary would be faster since they were already ordered initially.
There was a problem hiding this comment.
Interesting idea... Hard to know - we'd likely pay more in other accesses/lookups (e.g. when generating the fluent APIs, when removing ignorable/by-convention annotations...), who knows which way it goes.
I've seen lots of things around code generation here which aren't very optimized - that's probably OK. If we have indication that things are slow, at some point we can profile a bit and get rid of the worst offenders etc...
(BTW perf is also why I decided to discovery namespaces in scaffolding via a single pass along with generation, rather than the approach used in MigrationsCodeGenerator of a 2nd pass only for namespace discovery. Not sure how much all this is important...)
src/EFCore.Design/Scaffolding/Internal/CSharpDbContextGenerator.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Design/Scaffolding/Internal/CSharpDbContextGenerator.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Design/Scaffolding/Internal/CSharpDbContextGenerator.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Design/Scaffolding/Internal/CSharpDbContextGenerator.cs
Outdated
Show resolved
Hide resolved
roji
force-pushed
the
roji/CodeFragmentNamespaces
branch
from
a46bc8e to
b91f109
Compare
This reverts commit c4c2822.
|
Thanks @AndriySvyryd! |
@bricelam this is a draft for getting your general OK before going further... #25046 (custom namespaces in scaffolding) is fully implemented, while #23848 (type-qualified Fluent APIs in the model snapshot) is a bit more tricky/ugly.
Closes #25046
Closes #23848
Closes #25434