Conversation

Notices

1. Embed this notice
   samir, talks too much (samir@mastodon.functional.computer)'s status on Thursday, 26-Jan-2023 22:37:14 JST samir, talks too much

   Been thinking about code review as a bottleneck.

   What if we didn’t review production code or unit (fast) tests, BUT:

   * all code must be fully tested
   * all non-unit (slow) tests must be reviewed, and written before the implementation
   * any exceptions to the above (basically, untested code) need full review

   You can still review the rest, just not as a blocker.

   What do you think? Would this work? Would it encourage good quality software?

   • Embed this notice
     silverwizard (silverwizard@convenient.email)'s status on Thursday, 26-Jan-2023 22:37:13 JST silverwizard

     in reply to
     @samir I feel like this is a weird question in that blockers are important for writing quality software?
     
     You need to explain your code to a reviewer as part of writing it, and you know a reviewer will call you out for doing bad.
     
     Also - what you *meant* to do isn't tested. Just what you did.
   • Embed this notice
     samir, talks too much (samir@mastodon.functional.computer)'s status on Thursday, 26-Jan-2023 22:48:04 JST samir, talks too much

     in reply to

     • silverwizard

     @silverwizard I have worked in teams that wrote very high-quality software without blocking code review.

     My experience has been that many people, when faced with blocking code review, simply make so many changes at once that the reviewer takes a cursory glance and says, “I guess it’s fine”.

     I agree with you that what you meant to do isn’t tested. I’m a big fan of acceptance tests (which are typically slow), and I would very much prefer if those were still reviewed.

     silverwizard likes this.
   • Embed this notice
     silverwizard (silverwizard@convenient.email)'s status on Thursday, 26-Jan-2023 22:51:49 JST silverwizard

     in reply to

     • silverwizard
     @samir That seems like those two paragraphs are contradictory - not sure how to reconcile them?
   • Embed this notice
     silverwizard (silverwizard@convenient.email)'s status on Thursday, 26-Jan-2023 23:42:14 JST silverwizard

     in reply to

     • silverwizard
     @samir It just seems like you are saying people are writing quality code without review, and review is being done badly, then it's kinda hard to measure anything.
     
     The value of good review is when reviewers take their role seriously.
   • Embed this notice
     samir, talks too much (samir@mastodon.functional.computer)'s status on Thursday, 26-Jan-2023 23:42:15 JST samir, talks too much

     in reply to

     • silverwizard

     @silverwizard Can you explain the contradiction?

   • Embed this notice
     silverwizard (silverwizard@convenient.email)'s status on Friday, 27-Jan-2023 00:00:29 JST silverwizard

     in reply to

     • silverwizard
     @samir I guess I don't want code touching my release branch until all the quality steps are complete?
   • Embed this notice
     samir, talks too much (samir@mastodon.functional.computer)'s status on Friday, 27-Jan-2023 00:00:30 JST samir, talks too much

     in reply to

     • silverwizard

     @silverwizard I’m explaining my experiences, which of course may not be your experiences.

     I think I agree with you, but I’m not sure why the review needs to be blocking in that case.

   • Embed this notice
     silverwizard (silverwizard@convenient.email)'s status on Friday, 27-Jan-2023 00:11:06 JST silverwizard

     in reply to

     • silverwizard
     @samir So generally my processes have involved:
     Code is committed to a branch
     pipelines kick off unit tests
     Code review confirms code does what it means to and meets quality standards
     code hits develop branch
     integration tests are run, both automated and manual on staging servers
     released to prod
   • Embed this notice
     samir, talks too much (samir@mastodon.functional.computer)'s status on Friday, 27-Jan-2023 00:11:07 JST samir, talks too much

     in reply to

     • silverwizard

     @silverwizard That’s interesting. Can you define “quality” in this case?

     Is there a reason why you prefer manual review to automated review here?

   • Embed this notice
     samir, talks too much (samir@mastodon.functional.computer)'s status on Friday, 27-Jan-2023 00:11:07 JST samir, talks too much

     in reply to

     • silverwizard

     @silverwizard Also, can I assume you don’t have any kind of manual testing phase, then?