Follow up comments for code hygiene for crrev.com/c/6737287. [chromium/src : main]
Lei Zhang (Gerrit)
Lei Zhang added 2 comments![]( "Open in Gerrit")
Thanks for the follow-up. I defer to OWNERS.
input.size() - raw_input.size(), raw_input.data(),I think this is just raw_input.size(), no subtraction needed?
Related details
- Stephen Nusko
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Lei Zhang (Gerrit)
Lei Zhang added 1 comment![]( "Open in Gerrit")
#ifdef UNSAFE_BUFFERS_BUILDBTW, I think this can be removed, assuming the prior CL spanified everything or put the questionable bits in UNSAFE_TODOs.
Related details
- Stephen Nusko
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Stephen Nusko (Gerrit)
Stephen Nusko voted and added 3 comments![]( "Open in Gerrit")
Votes added by Stephen Nusko
| Commit-Queue | +1 |
Thanks for the follow-up. I defer to OWNERS.
This also looks to be the only one that slipped in.
I think this is just raw_input.size(), no subtraction needed?
Indeed, I think the CQ would have caught this, was to quick in just updating the size part.
input.size() - raw_input.size(), raw_input.data(),I think this is just raw_input.size(), no subtraction needed?
Indeed now that I look closer 😊, I assume a test would have caught this once it had the chance to run.
Related details
- Lei Zhang
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Stephen Nusko (Gerrit)
Stephen Nusko voted and added 2 comments![]( "Open in Gerrit")
Votes added by Stephen Nusko
| Auto-Submit | +1 |
2 comments
+ccameron@ from the initial review, a couple follow ups that were pointed out by Lei.
BTW, I think this can be removed, assuming the prior CL spanified everything or put the questionable bits in UNSAFE_TODOs.
There is an auto remover we run weekly, but also I see there is some indexing into a struct which is declared in third_party that we can't spanify which still doesn't have UNSAFE_TODO around it (so likely going to fail). Thomas is is currently in the process of running [unsafe_pragma_remover](https://source.chromium.org/chromium/chromium/src/+/main:tools/clang/unsafe_pragma_rewriter/;bpv=1) to switch all file level pragmas to line based UNSAFE_TODO so I'll let that get to this one.
Related details
- ccameron chromium
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
ccameron chromium (Gerrit)
ccameron chromium added 2 comments![]( "Open in Gerrit")
+1 on the fixes, -1 on removing the tests
void ApplyCrtcColorSpaceConversion(DrmWrapper* drm,This is used to verify that the various DRM properties were set to the correct values in the below tests.
Maybe this should be refactored to be moved to test-only parts of the codebase.
(It's also an evolutionary dead-end with the al work replacing it)
Related details
- Stephen Nusko
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Stephen Nusko (Gerrit)
Stephen Nusko voted and added 1 comment![]( "Open in Gerrit")
Votes added by Stephen Nusko
| Auto-Submit | +1 |
| Commit-Queue | +1 |
1 comment
void ApplyCrtcColorSpaceConversion(DrmWrapper* drm,This is used to verify that the various DRM properties were set to the correct values in the below tests.
Maybe this should be refactored to be moved to test-only parts of the codebase.
(It's also an evolutionary dead-end with the al work replacing it)
SGTM, will revert these files then and let you decide in future the approach. 😊
Related details
- ccameron chromium
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Stephen Nusko (Gerrit)
Stephen Nusko voted and added 1 comment![]( "Open in Gerrit")
Votes added by Stephen Nusko
| Commit-Queue | +1 |
1 comment
Mild ping @ccam...@chromium.org, I've removed the function so this is now just a bit of hygiene fix up.
Related details
- ccameron chromium
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |