{"id":874,"date":"2009-10-27T13:38:22","date_gmt":"2009-10-27T18:38:22","guid":{"rendered":"http:\/\/mikeconley.ca\/blog\/?p=874"},"modified":"2023-12-20T16:25:18","modified_gmt":"2023-12-20T21:25:18","slug":"code-reviews-and-predictive-impact-analysis","status":"publish","type":"post","link":"https:\/\/mikeconley.ca\/blog\/2009\/10\/27\/code-reviews-and-predictive-impact-analysis\/","title":{"rendered":"Code Reviews and Predictive Impact Analysis"},"content":{"rendered":"<p>A few posts ago, I mentioned what I think of as <a href=\"http:\/\/mikeconley.ca\/blog\/2009\/10\/08\/the-achilles-heel-of-light-weight-code-review\/\">the Achilles&#8217; Heel of light-weight code review<\/a>:\u00a0 the lack of feedback over dependencies that can\/will be impacted by a posted change.\u00a0 I believe this lack of feedback can potentially give software developers the false impression that proposed code is sound, and thus allow bugs to slip through the review process.\u00a0 This has happened more than once with <a href=\"http:\/\/www.markusproject.org\">the MarkUs project<\/a>, where we are using <a href=\"http:\/\/www.review-board.org\/\">ReviewBoard<\/a>.<\/p>\n<h2>Wouldn&#8217;t it be nice&#8230;<\/h2>\n<p>Imagine that you&#8217;re working on a &#8220;Library&#8221; Rails project.\u00a0 Imagine that you&#8217;re about to make an update to the Book model within the MVC framework:\u00a0 you&#8217;ve added a more efficient static class method to the Book model that allows you to check out large quantities of Books from the Library all at once, rather than one at a time.\u00a0 Cool.\u00a0 You update the BookController to use the new method, run your regression tests (which are admittedly incomplete, and pass with flying colours), and post your code for review.<\/p>\n<p>Your code review tool takes the change you&#8217;re suggesting, and notices a trend:\u00a0 in the past, when the &#8220;checkout&#8221; methods in the Book model have been updated, the BookController is usually changed, and <em>a particular area in en.yml locale file is usually updated too<\/em>.\u00a0 The code review tool notices that in this latest change, nothing has been changed in en.yml.<\/p>\n<p>The code review tool raises its eyebrow.\u00a0 &#8220;I wonder if they forgot something&#8230;&#8221;, it ponders.<\/p>\n<p>Now imagine that someone logs in to review the code.\u00a0 Along with the proposed changes, the code review tool suggests that the reviewer also take a peek at en.yml just in case the submitter has missed something.\u00a0 The reviewer notices that, yes, a translation string for an error message in en.yml no longer makes sense with the new method.\u00a0 The reviewer writes a comment about it, and submits the review.<\/p>\n<p>The reviewee looks at the review and goes, &#8220;Of course!\u00a0 How could I forget that?&#8221;, and updates the en.yml before updating the diff under review.<\/p>\n<p>Hm.\u00a0 It&#8217;s like a recommendation engine for code reviews&#8230;&#8221;by the way, have you checked&#8230;?&#8221;<\/p>\n<p>I wonder if this would be useful&#8230;<\/p>\n<h2>Mining Repositories for Predicting Impact Analysis<\/h2>\n<p>This area of research is really new to me, so bear with me as I stumble through it.<\/p>\n<p>It seems like it should be possible to predict what methods\/files have dependencies on other files based on static analysis, as well as VCS repository mining.\u00a0 I believe <a href=\"http:\/\/www.cs.ubc.ca\/labs\/spl\/papers\/2002\/icse02-feat.pdf\">this<\/a> <a href=\"http:\/\/74.125.155.132\/scholar?q=cache:9MoooL2SyggJ:scholar.google.com\/+%22Impact+Analysis+by+Mining+Software+and+Change+Request+Repositories%22&amp;hl=en\">has<\/a> <a href=\"http:\/\/portal.acm.org\/citation.cfm?id=1271355\">been<\/a> <a href=\"http:\/\/portal.acm.org\/citation.cfm?id=1137983.1138009&amp;coll=portal&amp;dl=ACM&amp;CFID=58582647&amp;CFTOKEN=72485145\">tried<\/a> in various forms.<\/p>\n<p>But I don&#8217;t think anything like this has been integrated into a code review tool.\u00a0 <a href=\"http:\/\/mikeconley.ca\/blog\/2009\/10\/13\/treasure-hunting-and-research-idea-4\/\">Certainly not any of the ones that I listed earlier.<\/a><\/p>\n<p>I wonder if such a tool would be accurate&#8230;\u00a0 and, again, would it be useful?\u00a0 Could it help catch more of the bugs that the standard light-weight code review process misses?<\/p>\n<p>Thoughts?<\/p>\n","protected":false},"excerpt":{"rendered":"<p>A few posts ago, I mentioned what I think of as the Achilles&#8217; Heel of light-weight code review:\u00a0 the lack of feedback over dependencies that can\/will be impacted by a posted change.\u00a0 I believe this lack of feedback can potentially give software developers the false impression that proposed code is sound, and thus allow bugs [&hellip;]<\/p>\n","protected":false},"author":4,"featured_media":0,"comment_status":"open","ping_status":"open","sticky":false,"template":"","format":"standard","meta":{"jetpack_post_was_ever_published":false,"_jetpack_newsletter_access":"","_jetpack_dont_email_post_to_subs":false,"_jetpack_newsletter_tier_id":0,"_jetpack_memberships_contains_paywalled_content":false,"_jetpack_memberships_contains_paid_content":false,"footnotes":"","jetpack_publicize_message":"","jetpack_publicize_feature_enabled":true,"jetpack_social_post_already_shared":false,"jetpack_social_options":{"image_generator_settings":{"template":"highway","default_image_id":0,"font":"","enabled":false},"version":2}},"categories":[454,162],"tags":[501,547,546,550,549,545,229,548],"class_list":["post-874","post","type-post","status-publish","format-standard","hentry","category-code-reviews","category-ruby-on-rails-technology","tag-code-review","tag-feat","tag-gail-murphy","tag-markus","tag-msr","tag-predictive-impact-analysis","tag-subversion","tag-svn"],"jetpack_publicize_connections":[],"jetpack_featured_media_url":"","jetpack_shortlink":"https:\/\/wp.me\/prmTy-e6","jetpack_sharing_enabled":true,"_links":{"self":[{"href":"https:\/\/mikeconley.ca\/blog\/wp-json\/wp\/v2\/posts\/874","targetHints":{"allow":["GET"]}}],"collection":[{"href":"https:\/\/mikeconley.ca\/blog\/wp-json\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/mikeconley.ca\/blog\/wp-json\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/mikeconley.ca\/blog\/wp-json\/wp\/v2\/users\/4"}],"replies":[{"embeddable":true,"href":"https:\/\/mikeconley.ca\/blog\/wp-json\/wp\/v2\/comments?post=874"}],"version-history":[{"count":9,"href":"https:\/\/mikeconley.ca\/blog\/wp-json\/wp\/v2\/posts\/874\/revisions"}],"predecessor-version":[{"id":3211,"href":"https:\/\/mikeconley.ca\/blog\/wp-json\/wp\/v2\/posts\/874\/revisions\/3211"}],"wp:attachment":[{"href":"https:\/\/mikeconley.ca\/blog\/wp-json\/wp\/v2\/media?parent=874"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/mikeconley.ca\/blog\/wp-json\/wp\/v2\/categories?post=874"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/mikeconley.ca\/blog\/wp-json\/wp\/v2\/tags?post=874"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}