-
Notifications
You must be signed in to change notification settings - Fork 763
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
File IO abstraction part 6: Simplify SourceFileFactory
#16242
Conversation
Test this change out locally with the following install scripts (Action run 13061804772) VSCode
Azure CLI
|
{ | ||
[new Uri("file:///mod.bicep")] = moduleContent, | ||
[new Uri("file:///mod2.bicep")] = moduleContent, |
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.
There's a lot of duplicate code in these tests that could be further simplified, but that's outside the scope of this PR.
547d48a
to
0c91fd9
Compare
0c91fd9
to
3fb7bcc
Compare
Dotnet Test Results 78 files - 39 78 suites - 39 33m 30s ⏱️ - 13m 19s Results for commit f3cb2fc. ± Comparison against base commit 7a77c7f. This pull request removes 1807 and adds 638 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
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.
Extracting more changes for my main file I/O migration branch...
This PR introduces two key improvements:
Simplifying
SourceFileFactory
– It merges multiple factory methods into one and refines the source file creation logic, allowing the removal ofBicepCompilationManager.CreateSourceFile
.Reducing unnecessary
SourceFileFactory
usage – Many language server integration tests previously relied on SourceFileFactory to create source files, resulting in unnecessary parsing, even though they only needed the source file URI, contents, and line starts. This PR introduces a new helper test class,LanguageClientFile
, eliminating the need to callSourceFileFactory
and providing several benefits:SourceFileFactory
get reduced from about 100 to 29. This is particularly important for narrowing the file I/O migration scope.Microsoft Reviewers: Open in CodeFlow