Details
nix develop # optional, if cargo + yarn is already installed cd services/commtest cargo clean && cargo build
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
These paths weren't referenced in any CI gate, so they were overlooked.
In the future we might want to block new builds/tests until there's a diff up that includes them to CI?
Was able to patch in diff and go through Test Plan successfully!
Accepting to unblock, but please address feedback about comment before landing.
| services/commtest/build.rs | ||
|---|---|---|
| 18 | I think it'd be helpful to explain the "why" here instead of the "what" | |
Accepting to unblock, but please address feedback about comment before landing.
This comment was not addressed. @jon, it is critical that you pay attention to comments like this... otherwise, your reviewers simply cannot trust you to handle an accept-with-comments correctly.
In the future we might want to block new builds/tests until there's a diff up that includes them to CI?
As with all things, I think that depends. But I think it's a big "process smell" that we don't have CI set up correctly for a lot of workflows (e.g. unit tests). CI/CD is more than just having passing gates, it's about managing changes and correctness of code; you really need the CI process in place before you start ramming through a bunch of code for it to be effective.
I think it'd be helpful to explain the "why" here instead of the "what"
Agreed, looks like I forgot to follow up on this and then landed it after looking through the rest of the stack.
This comment was not addressed. @jon, it is critical that you pay attention to comments like this... otherwise, your reviewers simply cannot trust you to handle an accept-with-comments correctly.
yea that was my bad. In this case I read this revision first, and then read the others to see if there was any other comments, and forgot to address this.
However, I'm also human, and the diff work queue is there to help me from myself. The revision really should have been blocking until I addressed the comments.
I would really like to get away from "accepted, but not really". It's super error prone, especially with larger stacks. It's also annoying to have to triple check every revision in stack make sure "green is green"
| services/commtest/build.rs | ||
|---|---|---|
| 18 | Passed over this, addressed in https://phab.comm.dev/D5154 | |