-
Notifications
You must be signed in to change notification settings - Fork 673
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
Support building launch assets for NET6-NET9 projects #4349
Conversation
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.
Otherwise LGTM
src/omnisharp/protocol.ts
Outdated
} | ||
|
||
export function findNetCoreAppTargetFramework(project: MSBuildProject): TargetFramework { | ||
return project.TargetFrameworks.find(tf => tf.ShortName.startsWith('netcoreapp')); | ||
} | ||
|
||
export function findModernNetFrameworkTargetFramework(project: MSBuildProject): TargetFramework { | ||
let regexp = new RegExp('^net[5-6]'); |
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.
Instead of explicitly testing for '6', would it make any sense to change this to [0-9+]
and then convert the number to a string and accept anything >= 50?
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.
Then I would need to do additional filtering to exclude net[1-4], Maybe the simpler option is to check for net[5-9] which would buy us a few years.
Codecov Report
@@ Coverage Diff @@
## master #4349 +/- ##
==========================================
- Coverage 86.08% 85.99% -0.09%
==========================================
Files 60 60
Lines 1875 1878 +3
Branches 217 218 +1
==========================================
+ Hits 1614 1615 +1
- Misses 200 202 +2
Partials 61 61
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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.
🥳
Resolves #4346.