Diff Detail
- Repository
- rFIREFOXAUTOLAND firefox-autoland
- Branch
- HEAD
Event Timeline
Code analysis found 2 defects in diff 1151660:
- 1 defect found by clang-format
- 1 defect found by clang-format (Mozlint)
You can run this analysis locally with:
- ./mach lint --warnings --outgoing
- ./mach clang-format -p netwerk/sctp/datachannel/DataChannel.cpp
For your convenience, here is a patch that fixes all the clang-format defects (use it in your repository with hg import or git apply -p0).
If you see a problem in this automated review, please report it here.
You can view these defects in the Diff Detail section of Phabricator diff 1151660.
r+, with some nits/comments
| netwerk/sctp/datachannel/DataChannel.cpp | ||
|---|---|---|
| 992–994 | Do we need a bug ref here? | |
| 1510 | Do we need the std::move here? Should this return void? (see SendBuffer comments) | |
| 1514 | Same as SendMsg | |
| 1527 | Should this function be void? This function only ever returns 0 directly or returns the result of DataChannelConnection::SendDataMessage(...) which only ever returns 0. | |
| 1579 | Nit. This kind of seems fragile. It seems like it could receive the closure to chain off the mMessagesSentPromise as a parameter. Up to you. It is only used in two places. | |
This revision requires a Testing Policy Project Tag to be set before landing. Please apply one of testing-approved, testing-exception-unchanged, testing-exception-ui, testing-exception-elsewhere, testing-exception-other. Tip: this Firefox add-on makes it easy!
| netwerk/sctp/datachannel/DataChannel.cpp | ||
|---|---|---|
| 992–994 | Filed bug 1998966, will reference here. | |
| 1510 | We'd need either a move or a forward, and we had been using move, but I could convert if you really wanted. None of this can fail at this point, so yeah I'll change the return types. | |
| 1579 | I'm ambivalent between keeping this as a function, or just inlining it in the two places it is used. It could use a thread assert though. | |
Code analysis found 2 defects in diff 1152281:
- 1 defect found by clang-format
- 1 defect found by clang-format (Mozlint)
You can run this analysis locally with:
- ./mach lint --warnings --outgoing
- ./mach clang-format -p netwerk/sctp/datachannel/DataChannel.cpp
For your convenience, here is a patch that fixes all the clang-format defects (use it in your repository with hg import or git apply -p0).
If you see a problem in this automated review, please report it here.
You can view these defects in the Diff Detail section of Phabricator diff 1152281.
Code analysis found 2 defects in diff 1152306:
- 1 defect found by clang-format
- 1 defect found by clang-format (Mozlint)
You can run this analysis locally with:
- ./mach clang-format -p netwerk/sctp/datachannel/DataChannel.cpp
- ./mach lint --warnings --outgoing
For your convenience, here is a patch that fixes all the clang-format defects (use it in your repository with hg import or git apply -p0).
If you see a problem in this automated review, please report it here.
You can view these defects in the Diff Detail section of Phabricator diff 1152306.