-
-
Notifications
You must be signed in to change notification settings - Fork 9
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
Upgrade .NET from 6.0 to 8.0 #3046
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3046 +/- ##
==========================================
+ Coverage 74.98% 75.14% +0.16%
==========================================
Files 273 273
Lines 10397 10386 -11
Branches 1230 1230
==========================================
+ Hits 7796 7805 +9
+ Misses 2241 2223 -18
+ Partials 360 358 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment has been minimized.
This comment has been minimized.
? Any thoughts on why this went away? |
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 there might be one more TryGetValue where we don't use the out value that I missed. Otherwise this is looking good.
Reviewed 38 of 42 files at r1, 3 of 5 files at r2, 1 of 20 files at r5, 2 of 11 files at r6, 1 of 1 files at r8.
Reviewable status: 12 of 43 files reviewed, all discussions resolved (waiting on @imnasnainaec)
.github/workflows/backend.yml
line 121 at r1 (raw file):
github.com:443 objects.githubusercontent.com:443 ts-crl.ws.symantec.com:80
what? Hmm, codeql hits this? weird.
Code quote:
ts-crl.ws.symantec.com:80
Backend/Controllers/InviteController.cs
line 95 at r1 (raw file):
{ currentUser = user; if (!user.ProjectRoles.TryGetValue(projectId, out var _roleId))
This one doesn't make sense to me. Why is it better to get a value we don't use?
Backend/Controllers/ProjectController.cs
line 57 at r1 (raw file):
var allUsers = await _userRepo.GetAllUsers(); var projectUsers = allUsers.FindAll(user => user.ProjectRoles.TryGetValue(projectId, out var _roleId));
Also here, why get the value and not use it - this suggestion doesn't make sense to me.
Backend/Services/StatisticsService.cs
line 74 at r1 (raw file):
var allUsers = await _userRepo.GetAllUsers(); var projectUsers = allUsers.FindAll(user => user.ProjectRoles.TryGetValue(projectId, out var _roleId));
I prefer ContainsKey if we don't go on to lookup the value.
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.
Reviewable status: 12 of 43 files reviewed, all discussions resolved (waiting on @jasonleenaylor)
docs/user_guide/assets/licenses/backend_licenses.txt
line 34 at r1 (raw file):
Previously, jasonleenaylor (Jason Naylor) wrote…
? Any thoughts on why this went away?
That's puzzling. It disappears when I run on my Windows so I reran on Mac and got it back.
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.
Reviewed 3 of 42 files at r1, 2 of 5 files at r2, 7 of 7 files at r3, 16 of 20 files at r5, 8 of 11 files at r6, 3 of 4 files at r7, 1 of 1 files at r10, 1 of 1 files at r11, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @imnasnainaec)
Follows #3049, #3050, #3052
Resolves #3043
This change is