From 7ac33a726757084f464b2092e0c1a331ef46c6e6 Mon Sep 17 00:00:00 2001
From: Donnie Tognazzini <donnie.tognazzini@appfolio.com>
Date: Fri, 4 Dec 2015 17:13:59 -0800
Subject: [PATCH 01/15] Use a hash for the stewards value entry in the
 "stewards registry"

---
 app/mailers/user_mailer.rb                   |   4 +-
 app/views/user_mailer/notify.html.erb        |   2 +-
 app/views/user_mailer/notify.text.erb        |   2 +-
 lib/ladle/pull_handler.rb                    |  33 ++--
 lib/ladle/steward_notifier.rb                |   4 +-
 test/ladle/pull_handler_test.rb              | 180 ++++++++++---------
 test/ladle/steward_notifier_test.rb          |  26 +--
 test/mailers/previews/user_mailer_preview.rb |  26 +--
 test/mailers/user_mailer_test.rb             |  30 ++--
 9 files changed, 166 insertions(+), 141 deletions(-)

diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb
index d70be07..3bc47e8 100644
--- a/app/mailers/user_mailer.rb
+++ b/app/mailers/user_mailer.rb
@@ -9,9 +9,9 @@ def title
     end
   end
 
-  def notify(user:, repository:, pull_request:, steward_changes_views:)
+  def notify(user:, repository:, pull_request:, stewards_file_map:)
     @pull_request = PullRequestPresenter.new(pull_request)
-    @steward_changes_views = steward_changes_views
+    @stewards_file_map = stewards_file_map
     @user = user
 
     mail(to:      user.email,
diff --git a/app/views/user_mailer/notify.html.erb b/app/views/user_mailer/notify.html.erb
index 63e3c56..1276833 100644
--- a/app/views/user_mailer/notify.html.erb
+++ b/app/views/user_mailer/notify.html.erb
@@ -9,7 +9,7 @@
 <p>You were notified of this pull request because of the following changes:</p>
 
 <ul>
-<% @steward_changes_views.each do |steward_changes| %>
+<% @stewards_file_map.each do |stewards_file_path, steward_changes| %>
   <li>
     <p>Changes monitored by <%= steward_changes.stewards_file %>:</p>
     <ul>
diff --git a/app/views/user_mailer/notify.text.erb b/app/views/user_mailer/notify.text.erb
index fc8484c..7451016 100644
--- a/app/views/user_mailer/notify.text.erb
+++ b/app/views/user_mailer/notify.text.erb
@@ -10,7 +10,7 @@ You can view, comment on, or merge this pull request online at:
 
 You were notified of this pull request because of the following changes:
 
-<% @steward_changes_views.each do |steward_changes| %>
+<% @stewards_file_map.each do |stewards_file_path, steward_changes| %>
   Changes monitored by <%= steward_changes.stewards_file %>:
 
   <% steward_changes.changes.each do |change| %>
diff --git a/lib/ladle/pull_handler.rb b/lib/ladle/pull_handler.rb
index b7bbba0..4fbcbf9 100644
--- a/lib/ladle/pull_handler.rb
+++ b/lib/ladle/pull_handler.rb
@@ -18,7 +18,7 @@ def handle(pull_request)
       read_current_stewards(stewards_registry, pull_request_files, pr_info.head_sha)
       read_old_stewards(stewards_registry, pull_request_files, pr_info.base_sha)
 
-      collect_files(stewards_registry, pull_request_files)
+      stewards_registry = resolve_stewards_scope(stewards_registry, pull_request_files)
 
       if stewards_registry.empty?
         Rails.logger.info('No stewards found. Doing nothing.')
@@ -47,12 +47,12 @@ def register_stewards(registry, stewards_file_path, sha)
       stewards_file = StewardsFileParser.parse(contents)
 
       stewards_file.stewards.each do |steward_config|
-        registry[steward_config.github_username] ||= []
+        registry[steward_config.github_username] ||= {}
 
         changes_view = Ladle::StewardChangesView.new(stewards_file: stewards_file_path,
                                                      file_filter: steward_config.file_filter)
 
-        registry[steward_config.github_username] << changes_view
+        registry[steward_config.github_username][stewards_file_path.to_s] = changes_view
       end
     rescue Ladle::RemoteFileNotFound
       # Ignore - stewards files don't have to exist
@@ -60,21 +60,26 @@ def register_stewards(registry, stewards_file_path, sha)
       Rails.logger.error("Error parsing file #{stewards_file_path}: #{e.message}\n#{e.backtrace.join("\n")}")
     end
 
-    def collect_files(stewards_registry, pull_request_files)
-      stewards_registry.each_value do |steward_change_views|
-        steward_change_views.each do |change_view|
-          file_changes = pull_request_files.file_changes_in(change_view.stewards_file.dirname)
-          change_view.add_file_changes(file_changes)
-        end
+    def select_non_empty_change_views(stewards_file_map, pull_request_files)
+      stewards_file_map.reject do |stewards_file_path, change_view|
+        file_changes = pull_request_files.file_changes_in(change_view.stewards_file.dirname)
+        change_view.add_file_changes(file_changes)
+        change_view.empty?
+      end
+    end
 
-        steward_change_views.reject! do |change_view|
-          change_view.empty?
+    def resolve_stewards_scope(stewards_registry, pull_request_files)
+      output = {}
+
+      stewards_registry.each do |github_username, stewards_file_map|
+        non_empty_file_map = select_non_empty_change_views(stewards_file_map, pull_request_files)
+
+        unless non_empty_file_map.empty?
+          output[github_username] = non_empty_file_map
         end
       end
 
-      stewards_registry.reject! do |_, steward_change_views|
-        steward_change_views.empty?
-      end
+      output
     end
   end
 end
diff --git a/lib/ladle/steward_notifier.rb b/lib/ladle/steward_notifier.rb
index 00ff944..a0e4a4e 100644
--- a/lib/ladle/steward_notifier.rb
+++ b/lib/ladle/steward_notifier.rb
@@ -25,8 +25,8 @@ def user_has_been_notified?(user)
       user.notifications.where(pull_request: @pull_request).exists?
     end
 
-    def send_email(user, steward_changes_views)
-      UserMailer.notify(user: user, repository: @repository_name, pull_request: @pull_request, steward_changes_views: steward_changes_views).deliver_now
+    def send_email(user, stewards_file_map)
+      UserMailer.notify(user: user, repository: @repository_name, pull_request: @pull_request, stewards_file_map: stewards_file_map).deliver_now
     end
 
     def create_notification(users)
diff --git a/test/ladle/pull_handler_test.rb b/test/ladle/pull_handler_test.rb
index 6a7bb07..4ee692f 100644
--- a/test/ladle/pull_handler_test.rb
+++ b/test/ladle/pull_handler_test.rb
@@ -73,25 +73,25 @@ class PullHandlerTest < ActiveSupport::TestCase
     notifier = mock
     notifier.expects(:notify)
       .with({
-              'xanderstrike'      => [
-                expected_sub_stewards_changes_view,
-                expected_stewards_changes_view
-              ],
-              'fadsfadsfadsfadsf' => [
-                expected_sub_stewards_changes_view
-              ],
-              'bob'               => [
-                build(:steward_changes_view,
-                      stewards_file: 'stewards.yml',
-                      file_filter: Ladle::FileFilter.new(
-                                               include_patterns: ["**.rb"],
-                                               exclude_patterns: ["**.txt"]
-                      ),
-                      changes:       [
-                                       build(:file_change, status: :added, file: 'one.rb', additions: 1),
-                                       build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1, deletions: 1),
-                                     ])
-              ]
+              'xanderstrike'      => {
+                'sub/stewards.yml' => expected_sub_stewards_changes_view,
+                'stewards.yml'     => expected_stewards_changes_view
+              },
+              'fadsfadsfadsfadsf' => {
+                'sub/stewards.yml' => expected_sub_stewards_changes_view
+              },
+              'bob'               => {
+                'stewards.yml' => build(:steward_changes_view,
+                                        stewards_file: 'stewards.yml',
+                                        file_filter:   Ladle::FileFilter.new(
+                                          include_patterns: ["**.rb"],
+                                          exclude_patterns: ["**.txt"]
+                                        ),
+                                        changes:       [
+                                                         build(:file_change, status: :added, file: 'one.rb', additions: 1),
+                                                         build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1, deletions: 1),
+                                                       ])
+              }
             })
 
     Ladle::PullHandler.new(client, notifier).handle(@pull_request)
@@ -171,24 +171,24 @@ class PullHandlerTest < ActiveSupport::TestCase
     notifier = mock
     notifier.expects(:notify)
       .with({
-              'xanderstrike'      => [
-                expected_sub_stewards_changes_view,
-                expected_stewards_changes_view,
-                expected_sub3_stewards_changes_view,
-              ],
-              'fadsfadsfadsfadsf' => [
-                expected_sub_stewards_changes_view
-              ],
-              'bob'               => [
-                expected_stewards_changes_view,
-                expected_sub3_stewards_changes_view
-              ],
-              'jeb'               => [
-                expected_sub_stewards_changes_view
-              ],
-              'hamburglar'        => [
-                build(:steward_changes_view, stewards_file: 'sub2/stewards.yml', changes: [build(:file_change, status: :removed, file: 'sub2/sandwich', deletions: 1)])
-              ],
+              'xanderstrike'      => {
+                'sub/stewards.yml'  => expected_sub_stewards_changes_view,
+                'stewards.yml'      => expected_stewards_changes_view,
+                'sub3/stewards.yml' => expected_sub3_stewards_changes_view,
+              },
+              'fadsfadsfadsfadsf' => {
+                'sub/stewards.yml' => expected_sub_stewards_changes_view
+              },
+              'bob'               => {
+                'stewards.yml'      => expected_stewards_changes_view,
+                'sub3/stewards.yml' => expected_sub3_stewards_changes_view
+              },
+              'jeb'               => {
+                'sub/stewards.yml' => expected_sub_stewards_changes_view
+              },
+              'hamburglar'        => {
+                'sub2/stewards.yml' => build(:steward_changes_view, stewards_file: 'sub2/stewards.yml', changes: [build(:file_change, status: :removed, file: 'sub2/sandwich', deletions: 1)])
+              },
             })
 
     Ladle::PullHandler.new(client, notifier).handle(@pull_request)
@@ -225,30 +225,29 @@ class PullHandlerTest < ActiveSupport::TestCase
         - bleh
     YAML
 
-    expected_stewards_changes_view = build(:steward_changes_view,
-                                           stewards_file: 'hello/stewards.yml',
-                                           changes:       [
-                                                            build(:file_change, status: :added, file: 'hello/kitty/what/che.txt', additions: 1),
-                                                            build(:file_change, status: :removed, file: 'hello/kitty/what/is/stewards.yml', deletions: 1),
-                                                          ])
+    expected_stewards_changes_view = {
+      'hello/stewards.yml' => build(:steward_changes_view,
+                                    stewards_file: 'hello/stewards.yml',
+                                    changes:       [
+                                                     build(:file_change, status: :added, file: 'hello/kitty/what/che.txt', additions: 1),
+                                                     build(:file_change, status: :removed, file: 'hello/kitty/what/is/stewards.yml', deletions: 1),
+                                                   ])
+    }
 
-    expected_sub_stewards_changes_view = build(:steward_changes_view,
-                                               stewards_file: 'hello/kitty/what/is/stewards.yml',
-                                               changes:       [
-                                                                build(:file_change, status: :removed, file: 'hello/kitty/what/is/stewards.yml', deletions: 1),
-                                                              ])
+    expected_sub_stewards_changes_view = {
+      'hello/kitty/what/is/stewards.yml' => build(:steward_changes_view,
+                                                  stewards_file: 'hello/kitty/what/is/stewards.yml',
+                                                  changes:       [
+                                                                   build(:file_change, status: :removed, file: 'hello/kitty/what/is/stewards.yml', deletions: 1),
+                                                                 ])
+    }
 
     notifier = mock
     notifier.stubs(:id).returns(1)
     notifier.expects(:notify)
       .with({
-              'xanderstrike' => [
-                expected_stewards_changes_view,
-                expected_sub_stewards_changes_view,
-              ],
-              'bleh'        => [
-                expected_sub_stewards_changes_view
-              ],
+              'xanderstrike' => expected_stewards_changes_view.merge(expected_sub_stewards_changes_view),
+              'bleh'         => expected_sub_stewards_changes_view
             })
 
     Ladle::PullHandler.new(client, notifier).handle(@pull_request)
@@ -275,22 +274,20 @@ class PullHandlerTest < ActiveSupport::TestCase
         - fadsfadsfadsfadsf
     YAML
 
-    expected_stewards_changes_view = build(:steward_changes_view,
-                                           stewards_file: 'stewards.yml',
-                                           changes:       [
-                                                            build(:file_change, status: :added, file: 'one.rb', additions: 1),
-                                                            build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1, deletions: 1),
-                                                          ])
+    expected_stewards_changes_view = {
+      'stewards.yml' => build(:steward_changes_view,
+                              stewards_file: 'stewards.yml',
+                              changes:       [
+                                               build(:file_change, status: :added, file: 'one.rb', additions: 1),
+                                               build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1, deletions: 1),
+                                             ])
+    }
 
     notifier = mock
     notifier.expects(:notify)
       .with({
-              'xanderstrike'      => [
-                expected_stewards_changes_view
-              ],
-              'fadsfadsfadsfadsf' => [
-                expected_stewards_changes_view
-              ],
+              'xanderstrike'      => expected_stewards_changes_view,
+              'fadsfadsfadsfadsf' => expected_stewards_changes_view,
             })
 
     Rails.logger.expects(:error).with(regexp_matches(/Error parsing file sub\/stewards.yml: Stewards file must contain a hash\n.*/))
@@ -327,26 +324,26 @@ class PullHandlerTest < ActiveSupport::TestCase
     notifier.stubs(:id).returns(1)
     notifier.expects(:notify)
       .with({
-              'xanderstrike' => [
-                build(:steward_changes_view,
-                      stewards_file: 'hello/stewards.yml',
-                      changes:       [
-                                       build(:file_change, status: :added, file: 'hello/kitty/what/is/your/favorite_food.yml', additions: 1),
-                                       build(:file_change, status: :added, file: 'hello/kitty/what/is/your/name.txt', additions: 1)
-                                     ]),
-              ],
+              'xanderstrike' => {
+                'hello/stewards.yml' => build(:steward_changes_view,
+                                              stewards_file: 'hello/stewards.yml',
+                                              changes:       [
+                                                               build(:file_change, status: :added, file: 'hello/kitty/what/is/your/favorite_food.yml', additions: 1),
+                                                               build(:file_change, status: :added, file: 'hello/kitty/what/is/your/name.txt', additions: 1)
+                                                             ]),
+              },
             })
 
     Ladle::PullHandler.new(client, notifier).handle(@pull_request)
   end
 
-  test "collect_files" do
+  test "resolve_stewards_scope" do
     registry = {
-      'xanderstrike' => [
-        build(:steward_changes_view, stewards_file: 'stewards.yml'),
-        build(:steward_changes_view, stewards_file: 'sub/stewards.yml'),
-        build(:steward_changes_view, stewards_file: 'sub3/stewards.yml'),
-      ]
+      'xanderstrike' => {
+        'stewards.yml'      => build(:steward_changes_view, stewards_file: 'stewards.yml'),
+        'sub/stewards.yml'  => build(:steward_changes_view, stewards_file: 'sub/stewards.yml'),
+        'sub3/stewards.yml' => build(:steward_changes_view, stewards_file: 'sub3/stewards.yml'),
+      }
     }
 
     changed_files = Ladle::ChangedFiles.new
@@ -358,7 +355,7 @@ class PullHandlerTest < ActiveSupport::TestCase
     changed_files.add_file_change(build(:file_change, file: 'sub3/stewards.yml'))
 
     handler = Ladle::PullHandler.new(@pull_request, mock('notifier'))
-    handler.send(:collect_files, registry, changed_files)
+    resolved_stewards_registry = handler.send(:resolve_stewards_scope, registry, changed_files)
 
     expected_changes_view = Ladle::StewardChangesView.new(
       stewards_file: 'stewards.yml',
@@ -371,7 +368,24 @@ class PullHandlerTest < ActiveSupport::TestCase
                        build(:file_change, file: 'sub3/stewards.yml')
                      ])
 
-    assert_equal expected_changes_view, registry['xanderstrike'].first
+    assert_equal expected_changes_view, resolved_stewards_registry['xanderstrike']['stewards.yml']
+
+    expected_changes_view = Ladle::StewardChangesView.new(
+      stewards_file: 'sub/stewards.yml',
+      changes:       [
+                       build(:file_change, file: 'sub/marine.rb'),
+                       build(:file_change, file: 'sub/stewards.yml'),
+                     ])
+
+    assert_equal expected_changes_view, resolved_stewards_registry['xanderstrike']['sub/stewards.yml']
+
+    expected_changes_view = Ladle::StewardChangesView.new(
+      stewards_file: 'sub3/stewards.yml',
+      changes:       [
+                       build(:file_change, file: 'sub3/stewards.yml')
+                     ])
+
+    assert_equal expected_changes_view, resolved_stewards_registry['xanderstrike']['sub3/stewards.yml']
   end
 
   private
diff --git a/test/ladle/steward_notifier_test.rb b/test/ladle/steward_notifier_test.rb
index 0d06a11..9ef4cca 100644
--- a/test/ladle/steward_notifier_test.rb
+++ b/test/ladle/steward_notifier_test.rb
@@ -90,17 +90,19 @@ class StewardNotifierTest < ActionController::TestCase
   private
 
   def create_steward_changes_views
-    [
-      Ladle::StewardChangesView.new(stewards_file: 'app/stewards.yml',
-                                    changes:       [
-                                                     build(:file_change, status: :removed, file: "app/removed_file.rb", deletions: 6),
-                                                     build(:file_change, status: :modified, file: "app/modified_file.rb", deletions: 3, additions: 3),
-                                                     build(:file_change, status: :added, file: "app/new_file.rb", additions: 6),
-                                                   ]),
-      Ladle::StewardChangesView.new(stewards_file: 'lib/closet/stewards.yml',
-                                    changes:       [
-                                                     build(:file_change, status: :added, file: "lib/closet/top_shelf/new_file.rb", additions: 6),
-                                                   ]),
-    ]
+    {
+      'app/stewards.yml'        => Ladle::StewardChangesView.new(
+        stewards_file: 'app/stewards.yml',
+        changes:       [
+                         build(:file_change, status: :removed, file: "app/removed_file.rb", deletions: 6),
+                         build(:file_change, status: :modified, file: "app/modified_file.rb", deletions: 3, additions: 3),
+                         build(:file_change, status: :added, file: "app/new_file.rb", additions: 6),
+                       ]),
+      'lib/closet/stewards.yml' => Ladle::StewardChangesView.new(
+        stewards_file: 'lib/closet/stewards.yml',
+        changes:       [
+                         build(:file_change, status: :added, file: "lib/closet/top_shelf/new_file.rb", additions: 6),
+                       ]),
+    }
   end
 end
diff --git a/test/mailers/previews/user_mailer_preview.rb b/test/mailers/previews/user_mailer_preview.rb
index 186c523..0b27e4f 100644
--- a/test/mailers/previews/user_mailer_preview.rb
+++ b/test/mailers/previews/user_mailer_preview.rb
@@ -31,17 +31,19 @@ def notify_without_description
   private
 
   def create_steward_changes_views
-    [
-      Ladle::StewardChangesView.new(stewards_file: 'app/stewards.yml',
-                                    changes:       [
-                                                     build(:file_change, status: :removed, file: "app/removed_file.rb", deletions: 6),
-                                                     build(:file_change, status: :modified, file: "app/modified_file.rb", deletions: 3, additions: 3),
-                                                     build(:file_change, status: :added, file: "app/new_file.rb", additions: 6),
-                                                   ]),
-      Ladle::StewardChangesView.new(stewards_file: 'lib/closet/stewards.yml',
-                                    changes:       [
-                                                     build(:file_change, status: :added, file: "lib/closet/top_shelf/new_file.rb", additions: 6),
-                                                   ]),
-    ]
+    {
+      'app/stewards.yml'        => Ladle::StewardChangesView.new(
+        stewards_file: 'app/stewards.yml',
+        changes:       [
+                         build(:file_change, status: :removed, file: "app/removed_file.rb", deletions: 6),
+                         build(:file_change, status: :modified, file: "app/modified_file.rb", deletions: 3, additions: 3),
+                         build(:file_change, status: :added, file: "app/new_file.rb", additions: 6),
+                       ]),
+      'lib/closet/stewards.yml' => Ladle::StewardChangesView.new(
+        stewards_file: 'lib/closet/stewards.yml',
+        changes:       [
+                         build(:file_change, status: :added, file: "lib/closet/top_shelf/new_file.rb", additions: 6),
+                       ]),
+    }
   end
 end
diff --git a/test/mailers/user_mailer_test.rb b/test/mailers/user_mailer_test.rb
index c69ac9d..da5e8f4 100644
--- a/test/mailers/user_mailer_test.rb
+++ b/test/mailers/user_mailer_test.rb
@@ -13,7 +13,7 @@ class UserMailerTest < ActionMailer::TestCase
                                 nil,
                                 nil
                               ),
-                              steward_changes_views: create_steward_changes_views).deliver_now
+                              stewards_file_map: create_steward_changes_views).deliver_now
 
     assert_not ActionMailer::Base.deliveries.empty?
 
@@ -32,7 +32,7 @@ class UserMailerTest < ActionMailer::TestCase
                                 'Hey ho!',
                                 'These changes are luminous',
                               ),
-                              steward_changes_views: create_steward_changes_views).deliver_now
+                              stewards_file_map: create_steward_changes_views).deliver_now
 
     assert_not ActionMailer::Base.deliveries.empty?
 
@@ -45,17 +45,19 @@ class UserMailerTest < ActionMailer::TestCase
   private
 
   def create_steward_changes_views
-    [
-      Ladle::StewardChangesView.new(stewards_file: 'app/stewards.yml',
-                                    changes:       [
-                                                     build(:file_change, status: :removed, file: "app/removed_file.rb", deletions: 6),
-                                                     build(:file_change, status: :modified, file: "app/modified_file.rb", deletions: 3, additions: 3),
-                                                     build(:file_change, status: :added, file: "app/new_file.rb", additions: 6),
-                                                   ]),
-      Ladle::StewardChangesView.new(stewards_file: 'lib/closet/stewards.yml',
-                                    changes:       [
-                                                     build(:file_change, status: :added, file: "lib/closet/top_shelf/new_file.rb", additions: 6),
-                                                   ]),
-    ]
+    {
+      'app/stewards.yml'        => Ladle::StewardChangesView.new(
+        stewards_file: 'app/stewards.yml',
+        changes:       [
+                         build(:file_change, status: :removed, file: "app/removed_file.rb", deletions: 6),
+                         build(:file_change, status: :modified, file: "app/modified_file.rb", deletions: 3, additions: 3),
+                         build(:file_change, status: :added, file: "app/new_file.rb", additions: 6),
+                       ]),
+      'lib/closet/stewards.yml' => Ladle::StewardChangesView.new(
+        stewards_file: 'lib/closet/stewards.yml',
+        changes:       [
+                         build(:file_change, status: :added, file: "lib/closet/top_shelf/new_file.rb", additions: 6),
+                       ]),
+    }
   end
 end

From ec8ab8bf6e800b6a342601ecd439c22d2f34943a Mon Sep 17 00:00:00 2001
From: Donnie Tognazzini <donnie.tognazzini@appfolio.com>
Date: Wed, 9 Dec 2015 18:54:30 -0800
Subject: [PATCH 02/15] Adding some inspect() impls to ease testing

---
 lib/ladle/file_change.rb          | 8 ++++++++
 lib/ladle/file_filter.rb          | 7 +++++++
 lib/ladle/steward_changes_view.rb | 8 ++++++++
 3 files changed, 23 insertions(+)

diff --git a/lib/ladle/file_change.rb b/lib/ladle/file_change.rb
index ca82991..d1ca1b9 100644
--- a/lib/ladle/file_change.rb
+++ b/lib/ladle/file_change.rb
@@ -45,5 +45,13 @@ def hash
     end
 
     alias eql? ==
+
+    def inspect
+      {
+        status: status_initial,
+        file: @file.to_s,
+        changes: changes_count,
+      }.inspect
+    end
   end
 end
diff --git a/lib/ladle/file_filter.rb b/lib/ladle/file_filter.rb
index ce71b5c..04b4ed6 100644
--- a/lib/ladle/file_filter.rb
+++ b/lib/ladle/file_filter.rb
@@ -28,6 +28,13 @@ def include?(file_pathname)
       matches_include && ! matches_exclude
     end
 
+    def inspect
+      {
+        include: @include_patterns,
+        exclude: @exclude_patterns,
+      }.inspect
+    end
+
     protected
 
     attr_reader :include_patterns, :exclude_patterns
diff --git a/lib/ladle/steward_changes_view.rb b/lib/ladle/steward_changes_view.rb
index a51aff4..d914c4f 100644
--- a/lib/ladle/steward_changes_view.rb
+++ b/lib/ladle/steward_changes_view.rb
@@ -33,5 +33,13 @@ def hash
     end
 
     alias eql? ==
+
+    def inspect
+      {
+        file_filter: @file_filter,
+        stewards_file: @stewards_file.to_s,
+        changes: @changes,
+      }.inspect
+    end
   end
 end

From 110b33a82d2069e6ff76b412e4d7e1877e114b0f Mon Sep 17 00:00:00 2001
From: Donnie Tognazzini <donnie.tognazzini@appfolio.com>
Date: Thu, 10 Dec 2015 12:38:23 -0800
Subject: [PATCH 03/15] Change stewards file map values to a list of change
 views

---
 app/views/user_mailer/notify.html.erb        |  8 +--
 app/views/user_mailer/notify.text.erb        |  8 +--
 lib/ladle/pull_handler.rb                    |  5 +-
 test/ladle/pull_handler_test.rb              | 54 ++++++++++----------
 test/ladle/steward_notifier_test.rb          | 24 ++++-----
 test/mailers/previews/user_mailer_preview.rb | 28 +++++-----
 test/mailers/user_mailer_test.rb             | 24 ++++-----
 7 files changed, 78 insertions(+), 73 deletions(-)

diff --git a/app/views/user_mailer/notify.html.erb b/app/views/user_mailer/notify.html.erb
index 1276833..43aa0d0 100644
--- a/app/views/user_mailer/notify.html.erb
+++ b/app/views/user_mailer/notify.html.erb
@@ -9,15 +9,17 @@
 <p>You were notified of this pull request because of the following changes:</p>
 
 <ul>
-<% @stewards_file_map.each do |stewards_file_path, steward_changes| %>
+  <% @stewards_file_map.each do |stewards_file_path, steward_change_views| %>
+    <% steward_change_views.each do |steward_change_view| %>
   <li>
-    <p>Changes monitored by <%= steward_changes.stewards_file %>:</p>
+    <p>Changes monitored by <%= steward_change_view.stewards_file %>:</p>
     <ul>
-      <% steward_changes.changes.each do |change| %>
+      <% steward_change_view.changes.each do |change| %>
         <li><strong><%= change.status_initial %></strong> <%= change.file %> (<%= change.changes_count %>)</li>
       <% end %>
     </ul>
   </li>
+    <% end %>
   <% end %>
 </ul>
 
diff --git a/app/views/user_mailer/notify.text.erb b/app/views/user_mailer/notify.text.erb
index 7451016..b35c021 100644
--- a/app/views/user_mailer/notify.text.erb
+++ b/app/views/user_mailer/notify.text.erb
@@ -10,13 +10,15 @@ You can view, comment on, or merge this pull request online at:
 
 You were notified of this pull request because of the following changes:
 
-<% @stewards_file_map.each do |stewards_file_path, steward_changes| %>
-  Changes monitored by <%= steward_changes.stewards_file %>:
+<% @stewards_file_map.each do |stewards_file_path, steward_change_views| %>
+  <% steward_change_views.each do |steward_change_view| %>
+  Changes monitored by <%= steward_change_view.stewards_file %>:
 
-  <% steward_changes.changes.each do |change| %>
+  <% steward_change_view.changes.each do |change| %>
     * <%= change.status_initial %> <%= change.file %> (<%= change.changes_count %>)
   <% end %>
 
+  <% end %>
 <% end %>
 ---------------------------------------------
 You received this email from Ladle for account <%= @user.github_username %>. Update your notification settings at <%= edit_user_settings_url %>.
diff --git a/lib/ladle/pull_handler.rb b/lib/ladle/pull_handler.rb
index 4fbcbf9..13583ad 100644
--- a/lib/ladle/pull_handler.rb
+++ b/lib/ladle/pull_handler.rb
@@ -52,7 +52,7 @@ def register_stewards(registry, stewards_file_path, sha)
         changes_view = Ladle::StewardChangesView.new(stewards_file: stewards_file_path,
                                                      file_filter: steward_config.file_filter)
 
-        registry[steward_config.github_username][stewards_file_path.to_s] = changes_view
+        registry[steward_config.github_username][stewards_file_path.to_s] = [changes_view]
       end
     rescue Ladle::RemoteFileNotFound
       # Ignore - stewards files don't have to exist
@@ -61,7 +61,8 @@ def register_stewards(registry, stewards_file_path, sha)
     end
 
     def select_non_empty_change_views(stewards_file_map, pull_request_files)
-      stewards_file_map.reject do |stewards_file_path, change_view|
+      stewards_file_map.reject do |stewards_file_path, change_views|
+        change_view = change_views.first
         file_changes = pull_request_files.file_changes_in(change_view.stewards_file.dirname)
         change_view.add_file_changes(file_changes)
         change_view.empty?
diff --git a/test/ladle/pull_handler_test.rb b/test/ladle/pull_handler_test.rb
index 4ee692f..54eaebc 100644
--- a/test/ladle/pull_handler_test.rb
+++ b/test/ladle/pull_handler_test.rb
@@ -74,14 +74,14 @@ class PullHandlerTest < ActiveSupport::TestCase
     notifier.expects(:notify)
       .with({
               'xanderstrike'      => {
-                'sub/stewards.yml' => expected_sub_stewards_changes_view,
-                'stewards.yml'     => expected_stewards_changes_view
+                'sub/stewards.yml' => [expected_sub_stewards_changes_view],
+                'stewards.yml'     => [expected_stewards_changes_view]
               },
               'fadsfadsfadsfadsf' => {
-                'sub/stewards.yml' => expected_sub_stewards_changes_view
+                'sub/stewards.yml' => [expected_sub_stewards_changes_view]
               },
               'bob'               => {
-                'stewards.yml' => build(:steward_changes_view,
+                'stewards.yml' => [build(:steward_changes_view,
                                         stewards_file: 'stewards.yml',
                                         file_filter:   Ladle::FileFilter.new(
                                           include_patterns: ["**.rb"],
@@ -90,7 +90,7 @@ class PullHandlerTest < ActiveSupport::TestCase
                                         changes:       [
                                                          build(:file_change, status: :added, file: 'one.rb', additions: 1),
                                                          build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1, deletions: 1),
-                                                       ])
+                                                       ])]
               }
             })
 
@@ -172,22 +172,22 @@ class PullHandlerTest < ActiveSupport::TestCase
     notifier.expects(:notify)
       .with({
               'xanderstrike'      => {
-                'sub/stewards.yml'  => expected_sub_stewards_changes_view,
-                'stewards.yml'      => expected_stewards_changes_view,
-                'sub3/stewards.yml' => expected_sub3_stewards_changes_view,
+                'sub/stewards.yml'  => [expected_sub_stewards_changes_view],
+                'stewards.yml'      => [expected_stewards_changes_view],
+                'sub3/stewards.yml' => [expected_sub3_stewards_changes_view],
               },
               'fadsfadsfadsfadsf' => {
-                'sub/stewards.yml' => expected_sub_stewards_changes_view
+                'sub/stewards.yml' => [expected_sub_stewards_changes_view]
               },
               'bob'               => {
-                'stewards.yml'      => expected_stewards_changes_view,
-                'sub3/stewards.yml' => expected_sub3_stewards_changes_view
+                'stewards.yml'      => [expected_stewards_changes_view],
+                'sub3/stewards.yml' => [expected_sub3_stewards_changes_view]
               },
               'jeb'               => {
-                'sub/stewards.yml' => expected_sub_stewards_changes_view
+                'sub/stewards.yml' => [expected_sub_stewards_changes_view]
               },
               'hamburglar'        => {
-                'sub2/stewards.yml' => build(:steward_changes_view, stewards_file: 'sub2/stewards.yml', changes: [build(:file_change, status: :removed, file: 'sub2/sandwich', deletions: 1)])
+                'sub2/stewards.yml' => [build(:steward_changes_view, stewards_file: 'sub2/stewards.yml', changes: [build(:file_change, status: :removed, file: 'sub2/sandwich', deletions: 1)])]
               },
             })
 
@@ -226,20 +226,20 @@ class PullHandlerTest < ActiveSupport::TestCase
     YAML
 
     expected_stewards_changes_view = {
-      'hello/stewards.yml' => build(:steward_changes_view,
+      'hello/stewards.yml' => [build(:steward_changes_view,
                                     stewards_file: 'hello/stewards.yml',
                                     changes:       [
                                                      build(:file_change, status: :added, file: 'hello/kitty/what/che.txt', additions: 1),
                                                      build(:file_change, status: :removed, file: 'hello/kitty/what/is/stewards.yml', deletions: 1),
-                                                   ])
+                                                   ])]
     }
 
     expected_sub_stewards_changes_view = {
-      'hello/kitty/what/is/stewards.yml' => build(:steward_changes_view,
+      'hello/kitty/what/is/stewards.yml' => [build(:steward_changes_view,
                                                   stewards_file: 'hello/kitty/what/is/stewards.yml',
                                                   changes:       [
                                                                    build(:file_change, status: :removed, file: 'hello/kitty/what/is/stewards.yml', deletions: 1),
-                                                                 ])
+                                                                 ])]
     }
 
     notifier = mock
@@ -275,12 +275,12 @@ class PullHandlerTest < ActiveSupport::TestCase
     YAML
 
     expected_stewards_changes_view = {
-      'stewards.yml' => build(:steward_changes_view,
+      'stewards.yml' => [build(:steward_changes_view,
                               stewards_file: 'stewards.yml',
                               changes:       [
                                                build(:file_change, status: :added, file: 'one.rb', additions: 1),
                                                build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1, deletions: 1),
-                                             ])
+                                             ])]
     }
 
     notifier = mock
@@ -325,12 +325,12 @@ class PullHandlerTest < ActiveSupport::TestCase
     notifier.expects(:notify)
       .with({
               'xanderstrike' => {
-                'hello/stewards.yml' => build(:steward_changes_view,
+                'hello/stewards.yml' => [build(:steward_changes_view,
                                               stewards_file: 'hello/stewards.yml',
                                               changes:       [
                                                                build(:file_change, status: :added, file: 'hello/kitty/what/is/your/favorite_food.yml', additions: 1),
                                                                build(:file_change, status: :added, file: 'hello/kitty/what/is/your/name.txt', additions: 1)
-                                                             ]),
+                                                             ])],
               },
             })
 
@@ -340,9 +340,9 @@ class PullHandlerTest < ActiveSupport::TestCase
   test "resolve_stewards_scope" do
     registry = {
       'xanderstrike' => {
-        'stewards.yml'      => build(:steward_changes_view, stewards_file: 'stewards.yml'),
-        'sub/stewards.yml'  => build(:steward_changes_view, stewards_file: 'sub/stewards.yml'),
-        'sub3/stewards.yml' => build(:steward_changes_view, stewards_file: 'sub3/stewards.yml'),
+        'stewards.yml'      => [build(:steward_changes_view, stewards_file: 'stewards.yml')],
+        'sub/stewards.yml'  => [build(:steward_changes_view, stewards_file: 'sub/stewards.yml')],
+        'sub3/stewards.yml' => [build(:steward_changes_view, stewards_file: 'sub3/stewards.yml')],
       }
     }
 
@@ -368,7 +368,7 @@ class PullHandlerTest < ActiveSupport::TestCase
                        build(:file_change, file: 'sub3/stewards.yml')
                      ])
 
-    assert_equal expected_changes_view, resolved_stewards_registry['xanderstrike']['stewards.yml']
+    assert_equal [expected_changes_view], resolved_stewards_registry['xanderstrike']['stewards.yml']
 
     expected_changes_view = Ladle::StewardChangesView.new(
       stewards_file: 'sub/stewards.yml',
@@ -377,7 +377,7 @@ class PullHandlerTest < ActiveSupport::TestCase
                        build(:file_change, file: 'sub/stewards.yml'),
                      ])
 
-    assert_equal expected_changes_view, resolved_stewards_registry['xanderstrike']['sub/stewards.yml']
+    assert_equal [expected_changes_view], resolved_stewards_registry['xanderstrike']['sub/stewards.yml']
 
     expected_changes_view = Ladle::StewardChangesView.new(
       stewards_file: 'sub3/stewards.yml',
@@ -385,7 +385,7 @@ class PullHandlerTest < ActiveSupport::TestCase
                        build(:file_change, file: 'sub3/stewards.yml')
                      ])
 
-    assert_equal expected_changes_view, resolved_stewards_registry['xanderstrike']['sub3/stewards.yml']
+    assert_equal [expected_changes_view], resolved_stewards_registry['xanderstrike']['sub3/stewards.yml']
   end
 
   private
diff --git a/test/ladle/steward_notifier_test.rb b/test/ladle/steward_notifier_test.rb
index 9ef4cca..ef6927a 100644
--- a/test/ladle/steward_notifier_test.rb
+++ b/test/ladle/steward_notifier_test.rb
@@ -91,18 +91,18 @@ class StewardNotifierTest < ActionController::TestCase
 
   def create_steward_changes_views
     {
-      'app/stewards.yml'        => Ladle::StewardChangesView.new(
-        stewards_file: 'app/stewards.yml',
-        changes:       [
-                         build(:file_change, status: :removed, file: "app/removed_file.rb", deletions: 6),
-                         build(:file_change, status: :modified, file: "app/modified_file.rb", deletions: 3, additions: 3),
-                         build(:file_change, status: :added, file: "app/new_file.rb", additions: 6),
-                       ]),
-      'lib/closet/stewards.yml' => Ladle::StewardChangesView.new(
-        stewards_file: 'lib/closet/stewards.yml',
-        changes:       [
-                         build(:file_change, status: :added, file: "lib/closet/top_shelf/new_file.rb", additions: 6),
-                       ]),
+      'app/stewards.yml'        => [Ladle::StewardChangesView.new(
+                                      stewards_file: 'app/stewards.yml',
+                                      changes:       [
+                                                       build(:file_change, status: :removed, file: "app/removed_file.rb", deletions: 6),
+                                                       build(:file_change, status: :modified, file: "app/modified_file.rb", deletions: 3, additions: 3),
+                                                       build(:file_change, status: :added, file: "app/new_file.rb", additions: 6),
+                                                     ])],
+      'lib/closet/stewards.yml' => [Ladle::StewardChangesView.new(
+                                      stewards_file: 'lib/closet/stewards.yml',
+                                      changes:       [
+                                                       build(:file_change, status: :added, file: "lib/closet/top_shelf/new_file.rb", additions: 6),
+                                                     ])],
     }
   end
 end
diff --git a/test/mailers/previews/user_mailer_preview.rb b/test/mailers/previews/user_mailer_preview.rb
index 0b27e4f..4c16553 100644
--- a/test/mailers/previews/user_mailer_preview.rb
+++ b/test/mailers/previews/user_mailer_preview.rb
@@ -13,7 +13,7 @@ def notify
                         'Initial commit',
                         "We made these changes because things needed to be changed and we noticed and we had the means to make the changes, so we did."
                       ),
-                      steward_changes_views: create_steward_changes_views)
+                      stewards_file_map: create_steward_changes_views)
   end
 
   def notify_without_description
@@ -24,7 +24,7 @@ def notify_without_description
                         'Initial commit',
                         nil
                       ),
-                      steward_changes_views: create_steward_changes_views)
+                      stewards_file_map: create_steward_changes_views)
 
   end
 
@@ -32,18 +32,18 @@ def notify_without_description
 
   def create_steward_changes_views
     {
-      'app/stewards.yml'        => Ladle::StewardChangesView.new(
-        stewards_file: 'app/stewards.yml',
-        changes:       [
-                         build(:file_change, status: :removed, file: "app/removed_file.rb", deletions: 6),
-                         build(:file_change, status: :modified, file: "app/modified_file.rb", deletions: 3, additions: 3),
-                         build(:file_change, status: :added, file: "app/new_file.rb", additions: 6),
-                       ]),
-      'lib/closet/stewards.yml' => Ladle::StewardChangesView.new(
-        stewards_file: 'lib/closet/stewards.yml',
-        changes:       [
-                         build(:file_change, status: :added, file: "lib/closet/top_shelf/new_file.rb", additions: 6),
-                       ]),
+      'app/stewards.yml'        => [Ladle::StewardChangesView.new(
+                                      stewards_file: 'app/stewards.yml',
+                                      changes:       [
+                                                       build(:file_change, status: :removed, file: "app/removed_file.rb", deletions: 6),
+                                                       build(:file_change, status: :modified, file: "app/modified_file.rb", deletions: 3, additions: 3),
+                                                       build(:file_change, status: :added, file: "app/new_file.rb", additions: 6),
+                                                     ])],
+      'lib/closet/stewards.yml' => [Ladle::StewardChangesView.new(
+                                      stewards_file: 'lib/closet/stewards.yml',
+                                      changes:       [
+                                                       build(:file_change, status: :added, file: "lib/closet/top_shelf/new_file.rb", additions: 6),
+                                                     ])],
     }
   end
 end
diff --git a/test/mailers/user_mailer_test.rb b/test/mailers/user_mailer_test.rb
index da5e8f4..d4ea01c 100644
--- a/test/mailers/user_mailer_test.rb
+++ b/test/mailers/user_mailer_test.rb
@@ -46,18 +46,18 @@ class UserMailerTest < ActionMailer::TestCase
 
   def create_steward_changes_views
     {
-      'app/stewards.yml'        => Ladle::StewardChangesView.new(
-        stewards_file: 'app/stewards.yml',
-        changes:       [
-                         build(:file_change, status: :removed, file: "app/removed_file.rb", deletions: 6),
-                         build(:file_change, status: :modified, file: "app/modified_file.rb", deletions: 3, additions: 3),
-                         build(:file_change, status: :added, file: "app/new_file.rb", additions: 6),
-                       ]),
-      'lib/closet/stewards.yml' => Ladle::StewardChangesView.new(
-        stewards_file: 'lib/closet/stewards.yml',
-        changes:       [
-                         build(:file_change, status: :added, file: "lib/closet/top_shelf/new_file.rb", additions: 6),
-                       ]),
+      'app/stewards.yml'        => [Ladle::StewardChangesView.new(
+                                      stewards_file: 'app/stewards.yml',
+                                      changes:       [
+                                                       build(:file_change, status: :removed, file: "app/removed_file.rb", deletions: 6),
+                                                       build(:file_change, status: :modified, file: "app/modified_file.rb", deletions: 3, additions: 3),
+                                                       build(:file_change, status: :added, file: "app/new_file.rb", additions: 6),
+                                                     ])],
+      'lib/closet/stewards.yml' => [Ladle::StewardChangesView.new(
+                                      stewards_file: 'lib/closet/stewards.yml',
+                                      changes:       [
+                                                       build(:file_change, status: :added, file: "lib/closet/top_shelf/new_file.rb", additions: 6),
+                                                     ])],
     }
   end
 end

From 8a938e7a7494cca223c4affe5185b2305abba694 Mon Sep 17 00:00:00 2001
From: Donnie Tognazzini <donnie.tognazzini@appfolio.com>
Date: Wed, 9 Dec 2015 19:25:41 -0800
Subject: [PATCH 04/15] Switch PullHandler to look across base branch first and
 then merge in changes from non-removed stewards.yml files on the source
 branch. This lays the foundation for including changes across both branches
 in the notification emails.

---
 lib/ladle/changed_files.rb                  |   2 +-
 lib/ladle/pull_handler.rb                   |  81 ++++++++---
 test/ladle/changed_files_test.rb            |  16 +++
 test/ladle/github_repository_client_test.rb |  10 +-
 test/ladle/local_repository_client_test.rb  |  12 +-
 test/ladle/pull_handler_test.rb             | 142 +++++++++++---------
 6 files changed, 166 insertions(+), 97 deletions(-)

diff --git a/lib/ladle/changed_files.rb b/lib/ladle/changed_files.rb
index aa562da..ea713ae 100644
--- a/lib/ladle/changed_files.rb
+++ b/lib/ladle/changed_files.rb
@@ -44,7 +44,7 @@ def add_file_change(file_change)
       @directories[directory_name] ||= Directory.new(directory_name)
       @directories[directory_name].add_change(file_change)
 
-      if file_change.file.to_s =~ /stewards\.yml$/ && file_change.status != :added
+      if file_change.file.to_s =~ /stewards\.yml$/ && file_change.status != :removed
         @modified_stewards_files << file_change.file
       end
 
diff --git a/lib/ladle/pull_handler.rb b/lib/ladle/pull_handler.rb
index 13583ad..dafec61 100644
--- a/lib/ladle/pull_handler.rb
+++ b/lib/ladle/pull_handler.rb
@@ -13,12 +13,13 @@ def handle(pull_request)
 
       pull_request_files = @client.pull_request_files(pull_request.number)
 
-      stewards_registry = {}
+      stewards_from_base = read_stewards(pull_request_files, pr_info.base_sha)
+      stewards_from_base = resolve_stewards_scope(stewards_from_base, pull_request_files)
 
-      read_current_stewards(stewards_registry, pull_request_files, pr_info.head_sha)
-      read_old_stewards(stewards_registry, pull_request_files, pr_info.base_sha)
+      stewards_from_head = read_new_stewards(pull_request_files, pr_info.head_sha)
+      stewards_from_head = resolve_stewards_scope(stewards_from_head, pull_request_files)
 
-      stewards_registry = resolve_stewards_scope(stewards_registry, pull_request_files)
+      stewards_registry = combine_stewards_tree_maps(stewards_from_base, stewards_from_head)
 
       if stewards_registry.empty?
         Rails.logger.info('No stewards found. Doing nothing.')
@@ -30,29 +31,33 @@ def handle(pull_request)
 
     private
 
-    def read_current_stewards(registry, pull_request_files, pr_head)
-      pull_request_files.directories.each do |directory|
-        register_stewards(registry, directory.join('stewards.yml'), pr_head)
+    def read_new_stewards(pull_request_files, base_head)
+      stewards_user_tree_map = {} #StewardsUserTreeMap.new
+      pull_request_files.modified_stewards_files.each do |stewards_file_path|
+        register_stewards(stewards_user_tree_map, stewards_file_path, base_head)
       end
+      stewards_user_tree_map
     end
 
-    def read_old_stewards(registry, pull_request_files, parent_head)
-      pull_request_files.modified_stewards_files.each do |stewards_file_path|
-        register_stewards(registry, stewards_file_path, parent_head)
+    def read_stewards(pull_request_files, pr_head)
+      stewards_user_tree_map = {} #StewardsUserTreeMap.new
+      pull_request_files.directories.each do |directory|
+        register_stewards(stewards_user_tree_map, directory.join('stewards.yml'), pr_head)
       end
+
+      stewards_user_tree_map
     end
 
-    def register_stewards(registry, stewards_file_path, sha)
+    def register_stewards(stewards_user_tree_map, stewards_file_path, sha)
       contents = @client.contents(path: stewards_file_path.to_s, ref: sha)
       stewards_file = StewardsFileParser.parse(contents)
 
       stewards_file.stewards.each do |steward_config|
-        registry[steward_config.github_username] ||= {}
-
         changes_view = Ladle::StewardChangesView.new(stewards_file: stewards_file_path,
                                                      file_filter: steward_config.file_filter)
 
-        registry[steward_config.github_username][stewards_file_path.to_s] = [changes_view]
+        stewards_user_tree_map[steward_config.github_username] ||= {}
+        stewards_user_tree_map[steward_config.github_username][stewards_file_path.to_s] = [changes_view]
       end
     rescue Ladle::RemoteFileNotFound
       # Ignore - stewards files don't have to exist
@@ -60,8 +65,8 @@ def register_stewards(registry, stewards_file_path, sha)
       Rails.logger.error("Error parsing file #{stewards_file_path}: #{e.message}\n#{e.backtrace.join("\n")}")
     end
 
-    def select_non_empty_change_views(stewards_file_map, pull_request_files)
-      stewards_file_map.reject do |stewards_file_path, change_views|
+    def select_non_empty_change_views(steward_tree, pull_request_files)
+      steward_tree.reject do |stewards_file_path, change_views|
         change_view = change_views.first
         file_changes = pull_request_files.file_changes_in(change_view.stewards_file.dirname)
         change_view.add_file_changes(file_changes)
@@ -69,11 +74,11 @@ def select_non_empty_change_views(stewards_file_map, pull_request_files)
       end
     end
 
-    def resolve_stewards_scope(stewards_registry, pull_request_files)
+    def resolve_stewards_scope(stewards_user_tree_map, pull_request_files)
       output = {}
 
-      stewards_registry.each do |github_username, stewards_file_map|
-        non_empty_file_map = select_non_empty_change_views(stewards_file_map, pull_request_files)
+      stewards_user_tree_map.each do |github_username, steward_tree|
+        non_empty_file_map = select_non_empty_change_views(steward_tree, pull_request_files)
 
         unless non_empty_file_map.empty?
           output[github_username] = non_empty_file_map
@@ -82,5 +87,43 @@ def resolve_stewards_scope(stewards_registry, pull_request_files)
 
       output
     end
+
+    def combine_stewards_tree_maps(left_steward_tree_map, right_steward_tree_map)
+      combined = {}
+
+      github_usernames = (left_steward_tree_map.keys + right_steward_tree_map.keys).uniq
+      github_usernames.each do |github_username|
+        left_steward_tree  = left_steward_tree_map[github_username]
+        right_steward_tree = right_steward_tree_map[github_username]
+
+        combined[github_username] = combine_steward_trees(left_steward_tree, right_steward_tree)
+      end
+
+      combined
+    end
+
+    def combine_steward_trees(left_steward_tree, right_steward_tree)
+      if left_steward_tree.nil? || right_steward_tree.nil?
+        return right_steward_tree.presence || left_steward_tree.presence
+      end
+
+      combined = {}
+
+      all_stewards_file_paths = (left_steward_tree.keys + right_steward_tree.keys).uniq
+      all_stewards_file_paths.each do |stewards_file_path|
+        left_change_views  = left_steward_tree[stewards_file_path]
+        right_change_views = right_steward_tree[stewards_file_path]
+
+        left_change_views = left_change_views.first unless left_change_views.nil?
+        right_change_views = right_change_views.first unless right_change_views.nil?
+
+        # TODO - handle the case
+        raise "Handle this case" if ! left_change_views.nil? && !right_change_views.nil?
+
+        combined[stewards_file_path] = [left_change_views, right_change_views].reject(&:nil?)
+      end
+
+      combined
+    end
   end
 end
diff --git a/test/ladle/changed_files_test.rb b/test/ladle/changed_files_test.rb
index e94bc66..3dd0709 100644
--- a/test/ladle/changed_files_test.rb
+++ b/test/ladle/changed_files_test.rb
@@ -42,6 +42,22 @@ class ChangedFilesTest < ActiveSupport::TestCase
     assert_equal [change1, change2, change4, change3], changed_files.file_changes_in(Pathname.new(""))
   end
 
+  test 'modified_stewards_files' do
+    changed_files = Ladle::ChangedFiles.new
+
+    added_stewards_file = build(:file_change, status: :added, file: "bob/loblaw/stewards.yml")
+    changed_files.add_file_change(added_stewards_file)
+    assert_equal [Pathname.new("bob/loblaw/stewards.yml")], changed_files.modified_stewards_files
+
+    removed_stewards_file = build(:file_change, status: :removed, file: "bob/loblaw/stewards.yml")
+    changed_files.add_file_change(removed_stewards_file)
+    assert_equal [Pathname.new("bob/loblaw/stewards.yml")], changed_files.modified_stewards_files
+
+    modified_stewards_file = build(:file_change, status: :modified, file: "bob/stewards.yml")
+    changed_files.add_file_change(modified_stewards_file)
+    assert_equal [Pathname.new("bob/loblaw/stewards.yml"), Pathname.new("bob/stewards.yml")], changed_files.modified_stewards_files
+  end
+
   test 'directories' do
     changed_files = Ladle::ChangedFiles.new
 
diff --git a/test/ladle/github_repository_client_test.rb b/test/ladle/github_repository_client_test.rb
index 14dc032..dd3614d 100644
--- a/test/ladle/github_repository_client_test.rb
+++ b/test/ladle/github_repository_client_test.rb
@@ -17,11 +17,11 @@ class GithubRepositoryClientTest < ActiveSupport::TestCase
                                                                                  sha: 'branch_head'
                                                                                },
                                                                                base: {
-                                                                                 sha: 'parent_head'
+                                                                                 sha: 'base_head'
                                                                                }
                                                                              })
 
-    assert_equal Ladle::PullRequestInfo.new('branch_head', 'parent_head'), @client.pull_request(12)
+    assert_equal Ladle::PullRequestInfo.new('branch_head', 'base_head'), @client.pull_request(12)
   end
 
   test "pull_request_files" do
@@ -71,7 +71,7 @@ class GithubRepositoryClientTest < ActiveSupport::TestCase
         sha: 'branch_head'
       },
       base: {
-        sha: 'parent_head'
+        sha: 'base_head'
       }
     }
 
@@ -93,10 +93,10 @@ class GithubRepositoryClientTest < ActiveSupport::TestCase
         - fadsfadsfadsfadsf
     YAML
 
-    stub_stewards_file_contents(octokit_client, expected_result, path: 'sub/stewards.yml', ref: 'branch_head')
+    stub_stewards_file_contents(octokit_client, expected_result, path: 'sub/stewards.yml', ref: 'base_head')
 
     octokit_client.expects(:contents)
-      .with(@repository.name, path: 'stewards.yml', ref: 'branch_head')
+      .with(@repository.name, path: 'stewards.yml', ref: 'base_head')
       .raises(Octokit::NotFound)
 
     notifier = mock
diff --git a/test/ladle/local_repository_client_test.rb b/test/ladle/local_repository_client_test.rb
index eec84fa..4a7c982 100644
--- a/test/ladle/local_repository_client_test.rb
+++ b/test/ladle/local_repository_client_test.rb
@@ -7,11 +7,11 @@ class LocalRepositoryClientTest < ActiveSupport::TestCase
 
   setup do
     @repository = create(:repository)
-    @client     = Ladle::LocalRepositoryClient.new(Rails.root.to_s, base_ref: 'parent_head', head_ref: 'branch_head')
+    @client     = Ladle::LocalRepositoryClient.new(Rails.root.to_s, base_ref: 'base_head', head_ref: 'branch_head')
   end
 
   test 'pull_request' do
-    assert_equal Ladle::PullRequestInfo.new('branch_head', 'parent_head'), @client.pull_request(12)
+    assert_equal Ladle::PullRequestInfo.new('branch_head', 'base_head'), @client.pull_request(12)
   end
 
   test "pull_request_files" do
@@ -20,7 +20,7 @@ class LocalRepositoryClientTest < ActiveSupport::TestCase
       mock(status: :modified, new_file: {path: "sub/marine.rb"}),
     ]
     commit = mock
-    commit.expects(:diff).with('parent_head').returns(mock(deltas: deltas))
+    commit.expects(:diff).with('base_head').returns(mock(deltas: deltas))
 
     rugged_client = Rugged::Repository.any_instance
     rugged_client.expects(:lookup).with('branch_head').returns(commit)
@@ -90,7 +90,7 @@ class LocalRepositoryClientTest < ActiveSupport::TestCase
     rugged_client = Rugged::Repository.any_instance
     rugged_client.expects(:lookup).with("branch_head").when(handler_state.is('finding_files')).returns(commit)
 
-    commit.expects(:diff).with('parent_head').returns(mock(deltas: deltas)).when(handler_state.is('finding_files')).then(handler_state.is('finding_content'))
+    commit.expects(:diff).with('base_head').returns(mock(deltas: deltas)).when(handler_state.is('finding_files')).then(handler_state.is('finding_content'))
 
     content = <<-YAML
       stewards:
@@ -102,8 +102,8 @@ class LocalRepositoryClientTest < ActiveSupport::TestCase
     tree.expects(:walk_blobs).twice.multiple_yields(["",{name: "some_file.bleh"}], ["sub", {name: "stewards.yml", oid: "object_id"}])
 
     content_sequence = sequence('content')
-    rugged_client.expects(:lookup).with("branch_head").when(handler_state.is('finding_content')).in_sequence(content_sequence).returns(mock('commit', tree: tree))
-    rugged_client.expects(:lookup).with("branch_head").when(handler_state.is('finding_content')).in_sequence(content_sequence).returns(mock('commit', tree: tree))
+    rugged_client.expects(:lookup).with("base_head").when(handler_state.is('finding_content')).in_sequence(content_sequence).returns(mock('commit', tree: tree))
+    rugged_client.expects(:lookup).with("base_head").when(handler_state.is('finding_content')).in_sequence(content_sequence).returns(mock('commit', tree: tree))
     rugged_client.expects(:lookup).with('object_id').returns(mock(content: content))
 
     notifier = mock
diff --git a/test/ladle/pull_handler_test.rb b/test/ladle/pull_handler_test.rb
index 54eaebc..5bdc23a 100644
--- a/test/ladle/pull_handler_test.rb
+++ b/test/ladle/pull_handler_test.rb
@@ -15,7 +15,7 @@ class PullHandlerTest < ActiveSupport::TestCase
   test 'does nothing when there are not stewards' do
     client = mock('repository_client')
     client.expects(:pull_request).with(@pull_request.number).returns(
-      Ladle::PullRequestInfo.new('branch_head', 'parent_head')
+      Ladle::PullRequestInfo.new('branch_head', 'base_head')
     )
 
     changed_files = Ladle::ChangedFiles.new
@@ -23,8 +23,8 @@ class PullHandlerTest < ActiveSupport::TestCase
     changed_files.add_file_change(build(:file_change, status: :modified, file: 'sub/marine.rb'))
     client.expects(:pull_request_files).with(@pull_request.number).returns(changed_files)
 
-    client.expects(:contents).with(path: 'sub/stewards.yml', ref: 'branch_head').raises(Ladle::RemoteFileNotFound)
-    client.expects(:contents).with(path: 'stewards.yml', ref: 'branch_head').raises(Ladle::RemoteFileNotFound)
+    client.expects(:contents).with(path: 'sub/stewards.yml', ref: 'base_head').raises(Ladle::RemoteFileNotFound)
+    client.expects(:contents).with(path: 'stewards.yml', ref: 'base_head').raises(Ladle::RemoteFileNotFound)
 
     logger_mock = mock
     logger_mock.expects(:info).with('No stewards found. Doing nothing.')
@@ -36,7 +36,7 @@ class PullHandlerTest < ActiveSupport::TestCase
   test 'notifies stewards' do
     client = mock('repository_client')
     client.expects(:pull_request).with(@pull_request.number).returns(
-      Ladle::PullRequestInfo.new('branch_head', 'parent_head')
+      Ladle::PullRequestInfo.new('branch_head', 'base_head')
     )
 
     changed_files = Ladle::ChangedFiles.new
@@ -44,13 +44,13 @@ class PullHandlerTest < ActiveSupport::TestCase
     changed_files.add_file_change(build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1, deletions: 1))
     client.expects(:pull_request_files).with(@pull_request.number).returns(changed_files)
 
-    client.expects(:contents).with(path: 'sub/stewards.yml', ref: 'branch_head').returns(<<-YAML)
+    client.expects(:contents).with(path: 'sub/stewards.yml', ref: 'base_head').returns(<<-YAML)
       stewards:
         - xanderstrike
         - fadsfadsfadsfadsf
     YAML
 
-    client.expects(:contents).with(path: 'stewards.yml', ref: 'branch_head').returns(<<-YAML)
+    client.expects(:contents).with(path: 'stewards.yml', ref: 'base_head').returns(<<-YAML)
       stewards:
         - github_username: bob
           include: "**.rb"
@@ -82,15 +82,16 @@ class PullHandlerTest < ActiveSupport::TestCase
               },
               'bob'               => {
                 'stewards.yml' => [build(:steward_changes_view,
-                                        stewards_file: 'stewards.yml',
-                                        file_filter:   Ladle::FileFilter.new(
-                                          include_patterns: ["**.rb"],
-                                          exclude_patterns: ["**.txt"]
-                                        ),
-                                        changes:       [
-                                                         build(:file_change, status: :added, file: 'one.rb', additions: 1),
-                                                         build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1, deletions: 1),
-                                                       ])]
+                                         stewards_file: 'stewards.yml',
+                                         file_filter:   Ladle::FileFilter.new(
+                                           include_patterns: ["**.rb"],
+                                           exclude_patterns: ["**.txt"]
+                                         ),
+                                         changes:       [
+                                                          build(:file_change, status: :added, file: 'one.rb', additions: 1),
+                                                          build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1, deletions: 1),
+                                                        ])
+                ]
               }
             })
 
@@ -100,7 +101,7 @@ class PullHandlerTest < ActiveSupport::TestCase
   test 'notifies old stewards' do
     client = mock('repository_client')
     client.expects(:pull_request).with(@pull_request.number).returns(
-      Ladle::PullRequestInfo.new('branch_head', 'parent_head')
+      Ladle::PullRequestInfo.new('branch_head', 'base_head')
     )
 
     changed_files = Ladle::ChangedFiles.new
@@ -109,40 +110,44 @@ class PullHandlerTest < ActiveSupport::TestCase
     changed_files.add_file_change(build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1))
     changed_files.add_file_change(build(:file_change, status: :modified, file: 'sub/stewards.yml', additions: 1))
     changed_files.add_file_change(build(:file_change, status: :removed, file: 'sub2/sandwich', deletions: 1))
+    changed_files.add_file_change(build(:file_change, status: :added, file: 'sub2/stewards.yml', additions: 1))
     changed_files.add_file_change(build(:file_change, status: :removed, file: 'sub3/stewards.yml', deletions: 1))
     client.expects(:pull_request_files).with(@pull_request.number).returns(changed_files)
 
-    client.expects(:contents).with(path: 'sub3/stewards.yml', ref: 'branch_head').raises(Ladle::RemoteFileNotFound)
+    # sub3/stewards.yml removed by branch
+    client.expects(:contents).with(path: 'sub3/stewards.yml', ref: 'base_head').returns(<<-YAML)
+      stewards:
+        - xanderstrike
+        - bob
+    YAML
+    client.expects(:contents).with(path: 'sub3/stewards.yml', ref: 'branch_head').never
 
+    # sub2/stewards.yml added by branch
+    client.expects(:contents).with(path: 'sub2/stewards.yml', ref: 'base_head').raises(Ladle::RemoteFileNotFound)
     client.expects(:contents).with(path: 'sub2/stewards.yml', ref: 'branch_head').returns(<<-YAML)
       stewards:
         - hamburglar
     YAML
 
-    client.expects(:contents).with(path: 'sub/stewards.yml', ref: 'branch_head').returns(<<-YAML)
+    # sub/stewards.yml exists on both, changed by branch
+    client.expects(:contents).with(path: 'sub/stewards.yml', ref: 'base_head').returns(<<-YAML)
       stewards:
-        - xanderstrike
-        - fadsfadsfadsfadsf
+        - jeb
     YAML
 
-    client.expects(:contents).with(path: 'stewards.yml', ref: 'branch_head').raises(Ladle::RemoteFileNotFound)
-
-    client.expects(:contents).with(path: 'stewards.yml', ref: 'parent_head').returns(<<-YAML)
+    client.expects(:contents).with(path: 'sub/stewards.yml', ref: 'branch_head').returns(<<-YAML)
       stewards:
         - xanderstrike
-        - bob
-    YAML
-
-    client.expects(:contents).with(path: 'sub/stewards.yml', ref: 'parent_head').returns(<<-YAML)
-      stewards:
-        - jeb
+        - fadsfadsfadsfadsf
     YAML
 
-    client.expects(:contents).with(path: 'sub3/stewards.yml', ref: 'parent_head').returns(<<-YAML)
+    # stewards.yml exists on base
+    client.expects(:contents).with(path: 'stewards.yml', ref: 'base_head').returns(<<-YAML)
       stewards:
         - xanderstrike
         - bob
     YAML
+    client.expects(:contents).with(path: 'stewards.yml', ref: 'branch_head').never
 
     expected_stewards_changes_view = build(:steward_changes_view,
                                            stewards_file: 'stewards.yml',
@@ -152,6 +157,7 @@ class PullHandlerTest < ActiveSupport::TestCase
                                                             build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1),
                                                             build(:file_change, status: :modified, file: 'sub/stewards.yml', additions: 1),
                                                             build(:file_change, status: :removed, file: 'sub2/sandwich', deletions: 1),
+                                                            build(:file_change, status: :added, file: 'sub2/stewards.yml', additions: 1),
                                                             build(:file_change, status: :removed, file: 'sub3/stewards.yml', deletions: 1)
                                                           ])
 
@@ -162,6 +168,13 @@ class PullHandlerTest < ActiveSupport::TestCase
                                                                 build(:file_change, status: :modified, file: 'sub/stewards.yml', additions: 1)
                                                               ])
 
+    expected_sub2_stewards_changes_view = build(:steward_changes_view,
+                                               stewards_file: 'sub2/stewards.yml',
+                                               changes:       [
+                                                                build(:file_change, status: :removed, file: 'sub2/sandwich', deletions: 1),
+                                                                build(:file_change, status: :added, file: 'sub2/stewards.yml', additions: 1)
+                                                              ])
+
     expected_sub3_stewards_changes_view = build(:steward_changes_view,
                                                 stewards_file: 'sub3/stewards.yml',
                                                 changes:       [
@@ -187,7 +200,7 @@ class PullHandlerTest < ActiveSupport::TestCase
                 'sub/stewards.yml' => [expected_sub_stewards_changes_view]
               },
               'hamburglar'        => {
-                'sub2/stewards.yml' => [build(:steward_changes_view, stewards_file: 'sub2/stewards.yml', changes: [build(:file_change, status: :removed, file: 'sub2/sandwich', deletions: 1)])]
+                'sub2/stewards.yml' => [expected_sub2_stewards_changes_view]
               },
             })
 
@@ -197,7 +210,7 @@ class PullHandlerTest < ActiveSupport::TestCase
   test 'notify - stewards file not in changes_view' do
     client = mock('repository_client')
     client.expects(:pull_request).with(@pull_request.number).returns(
-      Ladle::PullRequestInfo.new('branch_head', 'parent_head')
+      Ladle::PullRequestInfo.new('branch_head', 'base_head')
     )
 
     changed_files = Ladle::ChangedFiles.new
@@ -206,48 +219,45 @@ class PullHandlerTest < ActiveSupport::TestCase
     changed_files.add_file_change(build(:file_change, status: :removed, file: 'hello/kitty/what/is/stewards.yml', deletions: 1))
     client.expects(:pull_request_files).with(@pull_request.number).returns(changed_files)
 
-    client.expects(:contents).with(path: 'goodbye/kitty/stewards.yml', ref: 'branch_head').raises(Ladle::RemoteFileNotFound)
-    client.expects(:contents).with(path: 'goodbye/stewards.yml', ref: 'branch_head').raises(Ladle::RemoteFileNotFound)
-    client.expects(:contents).with(path: 'hello/kitty/what/is/stewards.yml', ref: 'branch_head').raises(Ladle::RemoteFileNotFound)
-    client.expects(:contents).with(path: 'hello/kitty/what/stewards.yml', ref: 'branch_head').raises(Ladle::RemoteFileNotFound)
-    client.expects(:contents).with(path: 'hello/kitty/stewards.yml', ref: 'branch_head').raises(Ladle::RemoteFileNotFound)
-
-    client.expects(:contents).with(path: 'hello/stewards.yml', ref: 'branch_head').returns(<<-YAML)
+    client.expects(:contents).with(path: 'goodbye/kitty/stewards.yml', ref: 'base_head').raises(Ladle::RemoteFileNotFound)
+    client.expects(:contents).with(path: 'goodbye/stewards.yml', ref: 'base_head').raises(Ladle::RemoteFileNotFound)
+    client.expects(:contents).with(path: 'hello/kitty/what/is/stewards.yml', ref: 'base_head').returns(<<-YAML)
       stewards:
         - xanderstrike
+        - bleh
     YAML
-
-    client.expects(:contents).with(path: 'stewards.yml', ref: 'branch_head').raises(Ladle::RemoteFileNotFound)
-
-    client.expects(:contents).with(path: 'hello/kitty/what/is/stewards.yml', ref: 'parent_head').returns(<<-YAML)
+    client.expects(:contents).with(path: 'hello/kitty/what/stewards.yml', ref: 'base_head').raises(Ladle::RemoteFileNotFound)
+    client.expects(:contents).with(path: 'hello/kitty/stewards.yml', ref: 'base_head').raises(Ladle::RemoteFileNotFound)
+    client.expects(:contents).with(path: 'hello/stewards.yml', ref: 'base_head').returns(<<-YAML)
       stewards:
         - xanderstrike
-        - bleh
     YAML
 
-    expected_stewards_changes_view = {
-      'hello/stewards.yml' => [build(:steward_changes_view,
+    client.expects(:contents).with(path: 'stewards.yml', ref: 'base_head').raises(Ladle::RemoteFileNotFound)
+
+    expected_stewards_changes_view = build(:steward_changes_view,
                                     stewards_file: 'hello/stewards.yml',
                                     changes:       [
                                                      build(:file_change, status: :added, file: 'hello/kitty/what/che.txt', additions: 1),
                                                      build(:file_change, status: :removed, file: 'hello/kitty/what/is/stewards.yml', deletions: 1),
-                                                   ])]
-    }
+                                                   ])
 
-    expected_sub_stewards_changes_view = {
-      'hello/kitty/what/is/stewards.yml' => [build(:steward_changes_view,
+    expected_sub_stewards_changes_view = build(:steward_changes_view,
                                                   stewards_file: 'hello/kitty/what/is/stewards.yml',
                                                   changes:       [
                                                                    build(:file_change, status: :removed, file: 'hello/kitty/what/is/stewards.yml', deletions: 1),
-                                                                 ])]
-    }
+                                                                 ])
 
     notifier = mock
-    notifier.stubs(:id).returns(1)
     notifier.expects(:notify)
       .with({
-              'xanderstrike' => expected_stewards_changes_view.merge(expected_sub_stewards_changes_view),
-              'bleh'         => expected_sub_stewards_changes_view
+              'xanderstrike' => {
+                'hello/stewards.yml' => [expected_stewards_changes_view],
+                'hello/kitty/what/is/stewards.yml' => [expected_sub_stewards_changes_view]
+              },
+              'bleh'         => {
+                'hello/kitty/what/is/stewards.yml' => [expected_sub_stewards_changes_view]
+              }
             })
 
     Ladle::PullHandler.new(client, notifier).handle(@pull_request)
@@ -256,7 +266,7 @@ class PullHandlerTest < ActiveSupport::TestCase
   test 'handle handles invalid stewards files ' do
     client = mock('repository_client')
     client.expects(:pull_request).with(@pull_request.number).returns(
-      Ladle::PullRequestInfo.new('branch_head', 'parent_head')
+      Ladle::PullRequestInfo.new('branch_head', 'base_head')
     )
 
     changed_files = Ladle::ChangedFiles.new
@@ -264,11 +274,11 @@ class PullHandlerTest < ActiveSupport::TestCase
     changed_files.add_file_change(build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1, deletions: 1))
     client.expects(:pull_request_files).with(@pull_request.number).returns(changed_files)
 
-    client.expects(:contents).with(path: 'sub/stewards.yml', ref: 'branch_head').returns(<<-YAML)
+    client.expects(:contents).with(path: 'sub/stewards.yml', ref: 'base_head').returns(<<-YAML)
       SOME WORDS
     YAML
 
-    client.expects(:contents).with(path: 'stewards.yml', ref: 'branch_head').returns(<<-YAML)
+    client.expects(:contents).with(path: 'stewards.yml', ref: 'base_head').returns(<<-YAML)
       stewards:
         - xanderstrike
         - fadsfadsfadsfadsf
@@ -298,7 +308,7 @@ class PullHandlerTest < ActiveSupport::TestCase
   test 'handle omits notifying of views/stewards without changes' do
     client = mock('repository_client')
     client.expects(:pull_request).with(@pull_request.number).returns(
-      Ladle::PullRequestInfo.new('branch_head', 'parent_head')
+      Ladle::PullRequestInfo.new('branch_head', 'base_head')
     )
 
     changed_files = Ladle::ChangedFiles.new
@@ -306,19 +316,19 @@ class PullHandlerTest < ActiveSupport::TestCase
     changed_files.add_file_change(build(:file_change, status: :added, file: 'hello/kitty/what/is/your/name.txt', additions: 1))
     client.expects(:pull_request_files).with(@pull_request.number).returns(changed_files)
 
-    client.expects(:contents).with(path: 'hello/kitty/what/is/your/stewards.yml', ref: 'branch_head').raises(Ladle::RemoteFileNotFound)
-    client.expects(:contents).with(path: 'hello/kitty/what/is/stewards.yml', ref: 'branch_head').raises(Ladle::RemoteFileNotFound)
-    client.expects(:contents).with(path: 'hello/kitty/what/stewards.yml', ref: 'branch_head').raises(Ladle::RemoteFileNotFound)
-    client.expects(:contents).with(path: 'hello/kitty/stewards.yml', ref: 'branch_head').raises(Ladle::RemoteFileNotFound)
+    client.expects(:contents).with(path: 'hello/kitty/what/is/your/stewards.yml', ref: 'base_head').raises(Ladle::RemoteFileNotFound)
+    client.expects(:contents).with(path: 'hello/kitty/what/is/stewards.yml', ref: 'base_head').raises(Ladle::RemoteFileNotFound)
+    client.expects(:contents).with(path: 'hello/kitty/what/stewards.yml', ref: 'base_head').raises(Ladle::RemoteFileNotFound)
+    client.expects(:contents).with(path: 'hello/kitty/stewards.yml', ref: 'base_head').raises(Ladle::RemoteFileNotFound)
 
-    client.expects(:contents).with(path: 'hello/stewards.yml', ref: 'branch_head').returns(<<-YAML)
+    client.expects(:contents).with(path: 'hello/stewards.yml', ref: 'base_head').returns(<<-YAML)
       stewards:
         - xanderstrike
         - github_username: bob
           include: "*.bin"
     YAML
 
-    client.expects(:contents).with(path: 'stewards.yml', ref: 'branch_head').raises(Ladle::RemoteFileNotFound)
+    client.expects(:contents).with(path: 'stewards.yml', ref: 'base_head').raises(Ladle::RemoteFileNotFound)
 
     notifier = mock
     notifier.stubs(:id).returns(1)
@@ -354,7 +364,7 @@ class PullHandlerTest < ActiveSupport::TestCase
     changed_files.add_file_change(build(:file_change, file: 'sub2/sandwich'))
     changed_files.add_file_change(build(:file_change, file: 'sub3/stewards.yml'))
 
-    handler = Ladle::PullHandler.new(@pull_request, mock('notifier'))
+    handler = Ladle::PullHandler.new(mock('client'), mock('notifier'))
     resolved_stewards_registry = handler.send(:resolve_stewards_scope, registry, changed_files)
 
     expected_changes_view = Ladle::StewardChangesView.new(

From 86ee8173bc1810dda3f3e0f5cae73209831173f7 Mon Sep 17 00:00:00 2001
From: Donnie Tognazzini <donnie.tognazzini@appfolio.com>
Date: Thu, 10 Dec 2015 11:29:26 -0800
Subject: [PATCH 05/15] Simplifying test setup by introducing StubbedRepoClient

---
 test/ladle/pull_handler_test.rb   | 96 ++++++++-----------------------
 test/ladle/stubbed_repo_client.rb | 41 +++++++++++++
 2 files changed, 66 insertions(+), 71 deletions(-)
 create mode 100644 test/ladle/stubbed_repo_client.rb

diff --git a/test/ladle/pull_handler_test.rb b/test/ladle/pull_handler_test.rb
index 5bdc23a..846a57c 100644
--- a/test/ladle/pull_handler_test.rb
+++ b/test/ladle/pull_handler_test.rb
@@ -5,6 +5,8 @@
 require 'ladle/changed_files'
 require 'ladle/file_filter'
 
+require 'ladle/stubbed_repo_client'
+
 class PullHandlerTest < ActiveSupport::TestCase
   setup do
     user = create(:user)
@@ -13,18 +15,11 @@ class PullHandlerTest < ActiveSupport::TestCase
   end
 
   test 'does nothing when there are not stewards' do
-    client = mock('repository_client')
-    client.expects(:pull_request).with(@pull_request.number).returns(
-      Ladle::PullRequestInfo.new('branch_head', 'base_head')
-    )
-
     changed_files = Ladle::ChangedFiles.new
     changed_files.add_file_change(build(:file_change, status: :added, file: 'one.rb'))
     changed_files.add_file_change(build(:file_change, status: :modified, file: 'sub/marine.rb'))
-    client.expects(:pull_request_files).with(@pull_request.number).returns(changed_files)
 
-    client.expects(:contents).with(path: 'sub/stewards.yml', ref: 'base_head').raises(Ladle::RemoteFileNotFound)
-    client.expects(:contents).with(path: 'stewards.yml', ref: 'base_head').raises(Ladle::RemoteFileNotFound)
+    client = Ladle::StubbedRepoClient.new(@pull_request.number, Ladle::PullRequestInfo.new('branch_head', 'base_head'), changed_files)
 
     logger_mock = mock
     logger_mock.expects(:info).with('No stewards found. Doing nothing.')
@@ -34,23 +29,19 @@ class PullHandlerTest < ActiveSupport::TestCase
   end
 
   test 'notifies stewards' do
-    client = mock('repository_client')
-    client.expects(:pull_request).with(@pull_request.number).returns(
-      Ladle::PullRequestInfo.new('branch_head', 'base_head')
-    )
-
     changed_files = Ladle::ChangedFiles.new
     changed_files.add_file_change(build(:file_change, status: :added, file: 'one.rb', additions: 1))
     changed_files.add_file_change(build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1, deletions: 1))
-    client.expects(:pull_request_files).with(@pull_request.number).returns(changed_files)
 
-    client.expects(:contents).with(path: 'sub/stewards.yml', ref: 'base_head').returns(<<-YAML)
+    client = Ladle::StubbedRepoClient.new(@pull_request.number, Ladle::PullRequestInfo.new('branch_head', 'base_head'), changed_files)
+
+    client.add_stewards_file(path: 'sub/stewards.yml', ref: 'base_head', contents: <<-YAML)
       stewards:
         - xanderstrike
         - fadsfadsfadsfadsf
     YAML
 
-    client.expects(:contents).with(path: 'stewards.yml', ref: 'base_head').returns(<<-YAML)
+    client.add_stewards_file(path: 'stewards.yml', ref: 'base_head', contents: <<-YAML)
       stewards:
         - github_username: bob
           include: "**.rb"
@@ -99,11 +90,6 @@ class PullHandlerTest < ActiveSupport::TestCase
   end
 
   test 'notifies old stewards' do
-    client = mock('repository_client')
-    client.expects(:pull_request).with(@pull_request.number).returns(
-      Ladle::PullRequestInfo.new('branch_head', 'base_head')
-    )
-
     changed_files = Ladle::ChangedFiles.new
     changed_files.add_file_change(build(:file_change, status: :removed, file: 'stewards.yml', deletions: 1))
     changed_files.add_file_change(build(:file_change, status: :added, file: 'one.rb', additions: 1))
@@ -112,42 +98,40 @@ class PullHandlerTest < ActiveSupport::TestCase
     changed_files.add_file_change(build(:file_change, status: :removed, file: 'sub2/sandwich', deletions: 1))
     changed_files.add_file_change(build(:file_change, status: :added, file: 'sub2/stewards.yml', additions: 1))
     changed_files.add_file_change(build(:file_change, status: :removed, file: 'sub3/stewards.yml', deletions: 1))
-    client.expects(:pull_request_files).with(@pull_request.number).returns(changed_files)
+
+    client = Ladle::StubbedRepoClient.new(@pull_request.number, Ladle::PullRequestInfo.new('branch_head', 'base_head'), changed_files)
 
     # sub3/stewards.yml removed by branch
-    client.expects(:contents).with(path: 'sub3/stewards.yml', ref: 'base_head').returns(<<-YAML)
+    client.add_stewards_file(path: 'sub3/stewards.yml', ref: 'base_head', contents: <<-YAML)
       stewards:
         - xanderstrike
         - bob
     YAML
-    client.expects(:contents).with(path: 'sub3/stewards.yml', ref: 'branch_head').never
 
     # sub2/stewards.yml added by branch
-    client.expects(:contents).with(path: 'sub2/stewards.yml', ref: 'base_head').raises(Ladle::RemoteFileNotFound)
-    client.expects(:contents).with(path: 'sub2/stewards.yml', ref: 'branch_head').returns(<<-YAML)
+    client.add_stewards_file(path: 'sub2/stewards.yml', ref: 'branch_head', contents: <<-YAML)
       stewards:
         - hamburglar
     YAML
 
     # sub/stewards.yml exists on both, changed by branch
-    client.expects(:contents).with(path: 'sub/stewards.yml', ref: 'base_head').returns(<<-YAML)
+    client.add_stewards_file(path: 'sub/stewards.yml', ref: 'base_head', contents: <<-YAML)
       stewards:
         - jeb
     YAML
 
-    client.expects(:contents).with(path: 'sub/stewards.yml', ref: 'branch_head').returns(<<-YAML)
+    client.add_stewards_file(path: 'sub/stewards.yml', ref: 'branch_head', contents: <<-YAML)
       stewards:
         - xanderstrike
         - fadsfadsfadsfadsf
     YAML
 
     # stewards.yml exists on base
-    client.expects(:contents).with(path: 'stewards.yml', ref: 'base_head').returns(<<-YAML)
+    client.add_stewards_file(path: 'stewards.yml', ref: 'base_head', contents: <<-YAML)
       stewards:
         - xanderstrike
         - bob
     YAML
-    client.expects(:contents).with(path: 'stewards.yml', ref: 'branch_head').never
 
     expected_stewards_changes_view = build(:steward_changes_view,
                                            stewards_file: 'stewards.yml',
@@ -208,33 +192,24 @@ class PullHandlerTest < ActiveSupport::TestCase
   end
 
   test 'notify - stewards file not in changes_view' do
-    client = mock('repository_client')
-    client.expects(:pull_request).with(@pull_request.number).returns(
-      Ladle::PullRequestInfo.new('branch_head', 'base_head')
-    )
-
     changed_files = Ladle::ChangedFiles.new
     changed_files.add_file_change(build(:file_change, status: :added, file: 'goodbye/kitty/sianara.txt', additions: 1))
     changed_files.add_file_change(build(:file_change, status: :added, file: 'hello/kitty/what/che.txt', additions: 1))
     changed_files.add_file_change(build(:file_change, status: :removed, file: 'hello/kitty/what/is/stewards.yml', deletions: 1))
-    client.expects(:pull_request_files).with(@pull_request.number).returns(changed_files)
 
-    client.expects(:contents).with(path: 'goodbye/kitty/stewards.yml', ref: 'base_head').raises(Ladle::RemoteFileNotFound)
-    client.expects(:contents).with(path: 'goodbye/stewards.yml', ref: 'base_head').raises(Ladle::RemoteFileNotFound)
-    client.expects(:contents).with(path: 'hello/kitty/what/is/stewards.yml', ref: 'base_head').returns(<<-YAML)
+    client = Ladle::StubbedRepoClient.new(@pull_request.number, Ladle::PullRequestInfo.new('branch_head', 'base_head'), changed_files)
+
+    client.add_stewards_file(path: 'hello/kitty/what/is/stewards.yml', ref: 'base_head', contents: <<-YAML)
       stewards:
         - xanderstrike
         - bleh
     YAML
-    client.expects(:contents).with(path: 'hello/kitty/what/stewards.yml', ref: 'base_head').raises(Ladle::RemoteFileNotFound)
-    client.expects(:contents).with(path: 'hello/kitty/stewards.yml', ref: 'base_head').raises(Ladle::RemoteFileNotFound)
-    client.expects(:contents).with(path: 'hello/stewards.yml', ref: 'base_head').returns(<<-YAML)
+
+    client.add_stewards_file(path: 'hello/stewards.yml', ref: 'base_head', contents: <<-YAML)
       stewards:
         - xanderstrike
     YAML
 
-    client.expects(:contents).with(path: 'stewards.yml', ref: 'base_head').raises(Ladle::RemoteFileNotFound)
-
     expected_stewards_changes_view = build(:steward_changes_view,
                                     stewards_file: 'hello/stewards.yml',
                                     changes:       [
@@ -264,21 +239,17 @@ class PullHandlerTest < ActiveSupport::TestCase
   end
 
   test 'handle handles invalid stewards files ' do
-    client = mock('repository_client')
-    client.expects(:pull_request).with(@pull_request.number).returns(
-      Ladle::PullRequestInfo.new('branch_head', 'base_head')
-    )
-
     changed_files = Ladle::ChangedFiles.new
     changed_files.add_file_change(build(:file_change, status: :added, file: 'one.rb', additions: 1))
     changed_files.add_file_change(build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1, deletions: 1))
-    client.expects(:pull_request_files).with(@pull_request.number).returns(changed_files)
 
-    client.expects(:contents).with(path: 'sub/stewards.yml', ref: 'base_head').returns(<<-YAML)
+    client = Ladle::StubbedRepoClient.new(@pull_request.number, Ladle::PullRequestInfo.new('branch_head', 'base_head'), changed_files)
+
+    client.add_stewards_file(path: 'sub/stewards.yml', ref: 'base_head', contents: <<-YAML)
       SOME WORDS
     YAML
 
-    client.expects(:contents).with(path: 'stewards.yml', ref: 'base_head').returns(<<-YAML)
+    client.add_stewards_file(path: 'stewards.yml', ref: 'base_head', contents: <<-YAML)
       stewards:
         - xanderstrike
         - fadsfadsfadsfadsf
@@ -306,30 +277,19 @@ class PullHandlerTest < ActiveSupport::TestCase
   end
 
   test 'handle omits notifying of views/stewards without changes' do
-    client = mock('repository_client')
-    client.expects(:pull_request).with(@pull_request.number).returns(
-      Ladle::PullRequestInfo.new('branch_head', 'base_head')
-    )
-
     changed_files = Ladle::ChangedFiles.new
     changed_files.add_file_change(build(:file_change, status: :added, file: 'hello/kitty/what/is/your/favorite_food.yml', additions: 1))
     changed_files.add_file_change(build(:file_change, status: :added, file: 'hello/kitty/what/is/your/name.txt', additions: 1))
-    client.expects(:pull_request_files).with(@pull_request.number).returns(changed_files)
 
-    client.expects(:contents).with(path: 'hello/kitty/what/is/your/stewards.yml', ref: 'base_head').raises(Ladle::RemoteFileNotFound)
-    client.expects(:contents).with(path: 'hello/kitty/what/is/stewards.yml', ref: 'base_head').raises(Ladle::RemoteFileNotFound)
-    client.expects(:contents).with(path: 'hello/kitty/what/stewards.yml', ref: 'base_head').raises(Ladle::RemoteFileNotFound)
-    client.expects(:contents).with(path: 'hello/kitty/stewards.yml', ref: 'base_head').raises(Ladle::RemoteFileNotFound)
+    client = Ladle::StubbedRepoClient.new(@pull_request.number, Ladle::PullRequestInfo.new('branch_head', 'base_head'), changed_files)
 
-    client.expects(:contents).with(path: 'hello/stewards.yml', ref: 'base_head').returns(<<-YAML)
+    client.add_stewards_file(path: 'hello/stewards.yml', ref: 'base_head', contents: <<-YAML)
       stewards:
         - xanderstrike
         - github_username: bob
           include: "*.bin"
     YAML
 
-    client.expects(:contents).with(path: 'stewards.yml', ref: 'base_head').raises(Ladle::RemoteFileNotFound)
-
     notifier = mock
     notifier.stubs(:id).returns(1)
     notifier.expects(:notify)
@@ -397,10 +357,4 @@ class PullHandlerTest < ActiveSupport::TestCase
 
     assert_equal [expected_changes_view], resolved_stewards_registry['xanderstrike']['sub3/stewards.yml']
   end
-
-  private
-
-  def stub_stewards_file_contents(client, contents, path:, ref:)
-    client.expects(:contents).with(path: path, ref: ref).returns(contents)
-  end
 end
diff --git a/test/ladle/stubbed_repo_client.rb b/test/ladle/stubbed_repo_client.rb
new file mode 100644
index 0000000..2bec36d
--- /dev/null
+++ b/test/ladle/stubbed_repo_client.rb
@@ -0,0 +1,41 @@
+module Ladle
+  class StubbedRepoClient
+    def initialize(pull_request_number, pull_request_info, changed_files)
+      @pull_request_number = pull_request_number
+      @pull_request_info   = pull_request_info
+      @changed_files       = changed_files
+
+      @repo                              = {}
+      @repo[@pull_request_info.head_sha] = {}
+      @repo[@pull_request_info.base_sha] = {}
+    end
+
+    def add_stewards_file(ref:, path:, contents:)
+      repo_at_ref = @repo[ref]
+      raise "Ref #{ref.inspect} not in repo" unless repo_at_ref
+
+      repo_at_ref[path] = contents
+    end
+
+    def contents(path:, ref:)
+      repo_at_ref = @repo[ref]
+      raise "Ref #{ref.inspect} not in repo" unless repo_at_ref
+
+      if repo_at_ref[path]
+        repo_at_ref[path]
+      else
+        raise Ladle::RemoteFileNotFound, "#{path} not found"
+      end
+    end
+
+    def pull_request(pr_number)
+      raise "Unexpected PR ##{pr_number}" unless pr_number == @pull_request_number
+      @pull_request_info
+    end
+
+    def pull_request_files(pr_number)
+      raise "Unexpected PR ##{pr_number}" unless pr_number == @pull_request_number
+      @changed_files
+    end
+  end
+end

From a8c0196657fc3ebaada546305b2fb77cf57d28a1 Mon Sep 17 00:00:00 2001
From: Donnie Tognazzini <donnie.tognazzini@appfolio.com>
Date: Thu, 10 Dec 2015 12:25:57 -0800
Subject: [PATCH 06/15] Adding and using deep_hash parameter matcher to mocha
 to ease testing

---
 Gemfile                           |  1 +
 Gemfile.lock                      |  2 +
 lib/ladle/file_change.rb          |  8 ----
 lib/ladle/file_filter.rb          |  8 ----
 lib/ladle/steward_changes_view.rb |  8 ----
 test/ladle/pull_handler_test.rb   | 22 ++++++-----
 test/mocha_deep_hash.rb           | 63 +++++++++++++++++++++++++++++++
 7 files changed, 78 insertions(+), 34 deletions(-)
 create mode 100644 test/mocha_deep_hash.rb

diff --git a/Gemfile b/Gemfile
index b3db2a3..17c194b 100644
--- a/Gemfile
+++ b/Gemfile
@@ -62,6 +62,7 @@ group :test do
   gem 'webmock'
   gem 'coveralls', require: false
   gem 'rugged', require: false
+  gem 'hashdiff', '0.2.2'
 end
 
 
diff --git a/Gemfile.lock b/Gemfile.lock
index 4f07386..00dbb3d 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -81,6 +81,7 @@ GEM
       multipart-post (>= 1.2, < 3)
     globalid (0.3.6)
       activesupport (>= 4.1.0)
+    hashdiff (0.2.2)
     hashie (3.4.2)
     http-cookie (1.0.2)
       domain_name (~> 0.5)
@@ -237,6 +238,7 @@ DEPENDENCIES
   devise
   devise-bootstrap-views
   factory_girl_rails (~> 4.0)
+  hashdiff (= 0.2.2)
   jbuilder (~> 2.0)
   jquery-rails
   mocha
diff --git a/lib/ladle/file_change.rb b/lib/ladle/file_change.rb
index d1ca1b9..ca82991 100644
--- a/lib/ladle/file_change.rb
+++ b/lib/ladle/file_change.rb
@@ -45,13 +45,5 @@ def hash
     end
 
     alias eql? ==
-
-    def inspect
-      {
-        status: status_initial,
-        file: @file.to_s,
-        changes: changes_count,
-      }.inspect
-    end
   end
 end
diff --git a/lib/ladle/file_filter.rb b/lib/ladle/file_filter.rb
index 04b4ed6..b428768 100644
--- a/lib/ladle/file_filter.rb
+++ b/lib/ladle/file_filter.rb
@@ -28,16 +28,8 @@ def include?(file_pathname)
       matches_include && ! matches_exclude
     end
 
-    def inspect
-      {
-        include: @include_patterns,
-        exclude: @exclude_patterns,
-      }.inspect
-    end
-
     protected
 
     attr_reader :include_patterns, :exclude_patterns
   end
-
 end
diff --git a/lib/ladle/steward_changes_view.rb b/lib/ladle/steward_changes_view.rb
index d914c4f..a51aff4 100644
--- a/lib/ladle/steward_changes_view.rb
+++ b/lib/ladle/steward_changes_view.rb
@@ -33,13 +33,5 @@ def hash
     end
 
     alias eql? ==
-
-    def inspect
-      {
-        file_filter: @file_filter,
-        stewards_file: @stewards_file.to_s,
-        changes: @changes,
-      }.inspect
-    end
   end
 end
diff --git a/test/ladle/pull_handler_test.rb b/test/ladle/pull_handler_test.rb
index 846a57c..3102b50 100644
--- a/test/ladle/pull_handler_test.rb
+++ b/test/ladle/pull_handler_test.rb
@@ -7,6 +7,8 @@
 
 require 'ladle/stubbed_repo_client'
 
+require 'mocha_deep_hash'
+
 class PullHandlerTest < ActiveSupport::TestCase
   setup do
     user = create(:user)
@@ -63,7 +65,7 @@ class PullHandlerTest < ActiveSupport::TestCase
                                                               ])
     notifier = mock
     notifier.expects(:notify)
-      .with({
+      .with(deep_hash({
               'xanderstrike'      => {
                 'sub/stewards.yml' => [expected_sub_stewards_changes_view],
                 'stewards.yml'     => [expected_stewards_changes_view]
@@ -84,7 +86,7 @@ class PullHandlerTest < ActiveSupport::TestCase
                                                         ])
                 ]
               }
-            })
+            }))
 
     Ladle::PullHandler.new(client, notifier).handle(@pull_request)
   end
@@ -167,7 +169,7 @@ class PullHandlerTest < ActiveSupport::TestCase
 
     notifier = mock
     notifier.expects(:notify)
-      .with({
+      .with(deep_hash({
               'xanderstrike'      => {
                 'sub/stewards.yml'  => [expected_sub_stewards_changes_view],
                 'stewards.yml'      => [expected_stewards_changes_view],
@@ -186,7 +188,7 @@ class PullHandlerTest < ActiveSupport::TestCase
               'hamburglar'        => {
                 'sub2/stewards.yml' => [expected_sub2_stewards_changes_view]
               },
-            })
+            }))
 
     Ladle::PullHandler.new(client, notifier).handle(@pull_request)
   end
@@ -225,7 +227,7 @@ class PullHandlerTest < ActiveSupport::TestCase
 
     notifier = mock
     notifier.expects(:notify)
-      .with({
+      .with(deep_hash({
               'xanderstrike' => {
                 'hello/stewards.yml' => [expected_stewards_changes_view],
                 'hello/kitty/what/is/stewards.yml' => [expected_sub_stewards_changes_view]
@@ -233,7 +235,7 @@ class PullHandlerTest < ActiveSupport::TestCase
               'bleh'         => {
                 'hello/kitty/what/is/stewards.yml' => [expected_sub_stewards_changes_view]
               }
-            })
+            }))
 
     Ladle::PullHandler.new(client, notifier).handle(@pull_request)
   end
@@ -266,10 +268,10 @@ class PullHandlerTest < ActiveSupport::TestCase
 
     notifier = mock
     notifier.expects(:notify)
-      .with({
+      .with(deep_hash({
               'xanderstrike'      => expected_stewards_changes_view,
               'fadsfadsfadsfadsf' => expected_stewards_changes_view,
-            })
+            }))
 
     Rails.logger.expects(:error).with(regexp_matches(/Error parsing file sub\/stewards.yml: Stewards file must contain a hash\n.*/))
 
@@ -293,7 +295,7 @@ class PullHandlerTest < ActiveSupport::TestCase
     notifier = mock
     notifier.stubs(:id).returns(1)
     notifier.expects(:notify)
-      .with({
+      .with(deep_hash({
               'xanderstrike' => {
                 'hello/stewards.yml' => [build(:steward_changes_view,
                                               stewards_file: 'hello/stewards.yml',
@@ -302,7 +304,7 @@ class PullHandlerTest < ActiveSupport::TestCase
                                                                build(:file_change, status: :added, file: 'hello/kitty/what/is/your/name.txt', additions: 1)
                                                              ])],
               },
-            })
+            }))
 
     Ladle::PullHandler.new(client, notifier).handle(@pull_request)
   end
diff --git a/test/mocha_deep_hash.rb b/test/mocha_deep_hash.rb
new file mode 100644
index 0000000..aaadec0
--- /dev/null
+++ b/test/mocha_deep_hash.rb
@@ -0,0 +1,63 @@
+require 'mocha/parameter_matchers/base'
+require 'hashdiff'
+require 'erb'
+
+module Mocha
+  module ParameterMatchers
+
+    # Matches +Hash+ using HashDiff, printing out differences on failure.
+    #
+    # @overload def deep_hash(expected)
+    #   @param [Object] expected hash.
+    #
+    # @return [DeepHash] parameter matcher.
+    #
+    # @see Expectation#with
+    #
+    # @example Actual parameter contains expected entry supplied as key and value.
+    #   object = mock()
+    #   object.expects(:method_1).with(deep_hash({a: 'blah'})
+    #   object.method_1({a: 'blah'})
+    #   # no error raised
+    #
+    # @example Actual parameter does not contain expected entry supplied as key and value.
+    #   object = mock()
+    #   object.expects(:method_1).with(deep_hash({a: 'blah'})
+    #   object.method_1({b: 'blah'})
+    #   # error raised, because method_1 was not called with a Hash matching {a: 'blah'}
+    #
+    def deep_hash(expected)
+      DeepHash.new(expected)
+    end
+
+    # Parameter matcher which matches when actual parameter contains expected +Hash+ entry.
+    class DeepHash < Base
+
+      def initialize(expected)
+        @expected = expected
+        @differences = []
+      end
+
+      def matches?(available_parameters)
+        parameter = available_parameters.shift
+
+        @differences = HashDiff.diff(@expected, parameter)
+
+        @differences == []
+      end
+
+      TEMPLATE = <<-ERB
+      deep_hash(<%= @expected.mocha_inspect %>)
+      Differences:
+        <% @differences.each do |difference| %>
+        <%= difference.inspect %>
+        <% end %>
+      ERB
+
+      def mocha_inspect
+        rb_balls = ERB.new(TEMPLATE)
+        rb_balls.result(binding)
+      end
+    end
+  end
+end

From 911218d8ea2f2bf0c1c1311b63997205a7dbe9e2 Mon Sep 17 00:00:00 2001
From: Donnie Tognazzini <donnie.tognazzini@appfolio.com>
Date: Thu, 10 Dec 2015 18:20:08 -0800
Subject: [PATCH 07/15] Include changes across branches when they differ

---
 lib/ladle/pull_handler.rb       |  9 ++--
 test/ladle/pull_handler_test.rb | 90 +++++++++++++++++++++++++++++++++
 2 files changed, 96 insertions(+), 3 deletions(-)

diff --git a/lib/ladle/pull_handler.rb b/lib/ladle/pull_handler.rb
index dafec61..d22d53b 100644
--- a/lib/ladle/pull_handler.rb
+++ b/lib/ladle/pull_handler.rb
@@ -117,10 +117,13 @@ def combine_steward_trees(left_steward_tree, right_steward_tree)
         left_change_views = left_change_views.first unless left_change_views.nil?
         right_change_views = right_change_views.first unless right_change_views.nil?
 
-        # TODO - handle the case
-        raise "Handle this case" if ! left_change_views.nil? && !right_change_views.nil?
+        all_changes = [left_change_views, right_change_views].reject(&:nil?)
 
-        combined[stewards_file_path] = [left_change_views, right_change_views].reject(&:nil?)
+        if all_changes.size == 2 && all_changes.first.changes == all_changes.last.changes
+          combined[stewards_file_path] = [all_changes.first]
+        else
+          combined[stewards_file_path] = all_changes
+        end
       end
 
       combined
diff --git a/test/ladle/pull_handler_test.rb b/test/ladle/pull_handler_test.rb
index 3102b50..a9a4611 100644
--- a/test/ladle/pull_handler_test.rb
+++ b/test/ladle/pull_handler_test.rb
@@ -91,6 +91,96 @@ class PullHandlerTest < ActiveSupport::TestCase
     Ladle::PullHandler.new(client, notifier).handle(@pull_request)
   end
 
+  test 'notifies steward from same file across branches' do
+    changed_files = Ladle::ChangedFiles.new
+    changed_files.add_file_change(build(:file_change, status: :added, file: 'file1.txt', additions: 1))
+    changed_files.add_file_change(build(:file_change, status: :added, file: 'file2.txt', additions: 1))
+    changed_files.add_file_change(build(:file_change, status: :modified, file: 'stewards.yml', additions: 1))
+
+    client = Ladle::StubbedRepoClient.new(@pull_request.number, Ladle::PullRequestInfo.new('branch_head', 'base_head'), changed_files)
+
+    client.add_stewards_file(path: 'stewards.yml', ref: 'base_head', contents: <<-YAML)
+      stewards:
+        - github_username: someguy
+          include: file1.txt
+    YAML
+
+    client.add_stewards_file(path: 'stewards.yml', ref: 'branch_head', contents: <<-YAML)
+      stewards:
+        - github_username: someguy
+          include: file2.txt
+    YAML
+
+    base_changes = build(:steward_changes_view,
+                         stewards_file: 'stewards.yml',
+                         file_filter:   Ladle::FileFilter.new(
+                           include_patterns: ["file1.txt"]
+                         ),
+                         changes:       [
+                                          build(:file_change, status: :added, file: 'file1.txt', additions: 1)
+                                        ])
+
+    branch_changes = build(:steward_changes_view,
+                           stewards_file: 'stewards.yml',
+                           file_filter:   Ladle::FileFilter.new(
+                             include_patterns: ["file2.txt"]
+                           ),
+                           changes:       [
+                                            build(:file_change, status: :added, file: 'file2.txt', additions: 1)
+                                          ])
+
+    notifier = mock
+    notifier.expects(:notify)
+      .with(deep_hash({
+                        'someguy' => {
+                          'stewards.yml' => [
+                            base_changes,
+                            branch_changes,
+                          ],
+                        }}))
+
+    Ladle::PullHandler.new(client, notifier).handle(@pull_request)
+  end
+
+  test 'notifies steward from same file across branches - remove duplicates' do
+    changed_files = Ladle::ChangedFiles.new
+    changed_files.add_file_change(build(:file_change, status: :added, file: 'file1.txt', additions: 1))
+    changed_files.add_file_change(build(:file_change, status: :added, file: 'file2.txt', additions: 1))
+    changed_files.add_file_change(build(:file_change, status: :modified, file: 'stewards.yml', additions: 1))
+
+    client = Ladle::StubbedRepoClient.new(@pull_request.number, Ladle::PullRequestInfo.new('branch_head', 'base_head'), changed_files)
+
+    client.add_stewards_file(path: 'stewards.yml', ref: 'base_head', contents: <<-YAML)
+      stewards:
+        - github_username: someguy
+          include: file1.txt
+    YAML
+
+    client.add_stewards_file(path: 'stewards.yml', ref: 'branch_head', contents: <<-YAML)
+      stewards:
+        - github_username: someguy
+          include: file1.*
+    YAML
+
+    changes = build(:steward_changes_view,
+                    stewards_file: 'stewards.yml',
+                    file_filter:   Ladle::FileFilter.new(
+                      include_patterns: ["file1.txt"]
+                    ),
+                    changes:       [
+                                     build(:file_change, status: :added, file: 'file1.txt', additions: 1)
+                                   ])
+
+    notifier = mock
+    notifier.expects(:notify)
+      .with(deep_hash({
+                        'someguy' => {
+                          'stewards.yml' => [changes],
+                        }}))
+
+    Ladle::PullHandler.new(client, notifier).handle(@pull_request)
+  end
+
   test 'notifies old stewards' do
     changed_files = Ladle::ChangedFiles.new
     changed_files.add_file_change(build(:file_change, status: :removed, file: 'stewards.yml', deletions: 1))

From bf9a4ce6a67b5c67fd9c6d2c6e5c29c3f978227c Mon Sep 17 00:00:00 2001
From: Donnie Tognazzini <donnie.tognazzini@appfolio.com>
Date: Mon, 14 Dec 2015 15:04:08 -0800
Subject: [PATCH 08/15] Reusing test steward tree maps and extending test data
 to include same-file-across-multiple-branches case

---
 lib/ladle/test_data.rb                       | 27 ++++++++++++++++++++
 test/ladle/steward_notifier_test.rb          | 27 ++++----------------
 test/mailers/previews/user_mailer_preview.rb | 27 +++-----------------
 test/mailers/user_mailer_test.rb             | 24 +++--------------
 4 files changed, 39 insertions(+), 66 deletions(-)
 create mode 100644 lib/ladle/test_data.rb

diff --git a/lib/ladle/test_data.rb b/lib/ladle/test_data.rb
new file mode 100644
index 0000000..eed92ba
--- /dev/null
+++ b/lib/ladle/test_data.rb
@@ -0,0 +1,27 @@
+module Ladle
+  module TestData
+    extend FactoryGirl::Syntax::Methods
+
+    def self.create_steward_changes_views
+      {
+        'app/stewards.yml'        => [Ladle::StewardChangesView.new(
+                                        stewards_file: 'app/stewards.yml',
+                                        changes:       [
+                                                         build(:file_change, status: :removed, file: "app/removed_file.rb", deletions: 6),
+                                                         build(:file_change, status: :modified, file: "app/modified_file.rb", deletions: 3, additions: 3),
+                                                         build(:file_change, status: :added, file: "app/new_file.rb", additions: 6),
+                                                       ]),
+                                      Ladle::StewardChangesView.new(
+                                        stewards_file: 'app/stewards.yml',
+                                        changes:       [
+                                                         build(:file_change, status: :modified, file: "app/blah.rb", deletions: 3, additions: 3),
+                                                       ])],
+        'lib/closet/stewards.yml' => [Ladle::StewardChangesView.new(
+                                        stewards_file: 'lib/closet/stewards.yml',
+                                        changes:       [
+                                                         build(:file_change, status: :added, file: "lib/closet/top_shelf/new_file.rb", additions: 6),
+                                                       ])],
+      }
+    end
+  end
+end
diff --git a/test/ladle/steward_notifier_test.rb b/test/ladle/steward_notifier_test.rb
index ef6927a..723fa9a 100644
--- a/test/ladle/steward_notifier_test.rb
+++ b/test/ladle/steward_notifier_test.rb
@@ -1,12 +1,14 @@
 require 'test_helper'
 require 'ladle/steward_notifier'
 
+require 'ladle/test_data'
+
 class StewardNotifierTest < ActionController::TestCase
   setup do
     @steward_changes_views = {
-      'xanderstrike' => create_steward_changes_views,
-      'counterstrike'=> create_steward_changes_views,
-      'boop'         => create_steward_changes_views
+      'xanderstrike' => Ladle::TestData.create_steward_changes_views,
+      'counterstrike'=> Ladle::TestData.create_steward_changes_views,
+      'boop'         => Ladle::TestData.create_steward_changes_views
     }
     @pull_request = create(:pull_request, html_url: 'https://github.com/XanderStrike/test/pull/11')
     @notifier = Ladle::StewardNotifier.new('XanderStrike/test', @pull_request)
@@ -86,23 +88,4 @@ class StewardNotifierTest < ActionController::TestCase
 
     assert_equal nil, notification
   end
-
-  private
-
-  def create_steward_changes_views
-    {
-      'app/stewards.yml'        => [Ladle::StewardChangesView.new(
-                                      stewards_file: 'app/stewards.yml',
-                                      changes:       [
-                                                       build(:file_change, status: :removed, file: "app/removed_file.rb", deletions: 6),
-                                                       build(:file_change, status: :modified, file: "app/modified_file.rb", deletions: 3, additions: 3),
-                                                       build(:file_change, status: :added, file: "app/new_file.rb", additions: 6),
-                                                     ])],
-      'lib/closet/stewards.yml' => [Ladle::StewardChangesView.new(
-                                      stewards_file: 'lib/closet/stewards.yml',
-                                      changes:       [
-                                                       build(:file_change, status: :added, file: "lib/closet/top_shelf/new_file.rb", additions: 6),
-                                                     ])],
-    }
-  end
 end
diff --git a/test/mailers/previews/user_mailer_preview.rb b/test/mailers/previews/user_mailer_preview.rb
index 4c16553..58c05d3 100644
--- a/test/mailers/previews/user_mailer_preview.rb
+++ b/test/mailers/previews/user_mailer_preview.rb
@@ -1,7 +1,7 @@
 # Preview all emails at http://localhost:3000/rails/mailers/user_mailer
-class UserMailerPreview < ActionMailer::Preview
-  include FactoryGirl::Syntax::Methods
+require 'ladle/test_data'
 
+class UserMailerPreview < ActionMailer::Preview
   UserLike = Struct.new(:email, :github_username)
   PullRequestLike = Struct.new(:html_url, :title, :body)
 
@@ -13,7 +13,7 @@ def notify
                         'Initial commit',
                         "We made these changes because things needed to be changed and we noticed and we had the means to make the changes, so we did."
                       ),
-                      stewards_file_map: create_steward_changes_views)
+                      stewards_file_map: Ladle::TestData.create_steward_changes_views)
   end
 
   def notify_without_description
@@ -24,26 +24,7 @@ def notify_without_description
                         'Initial commit',
                         nil
                       ),
-                      stewards_file_map: create_steward_changes_views)
-
-  end
-
-  private
+                      stewards_file_map: Ladle::TestData.create_steward_changes_views)
 
-  def create_steward_changes_views
-    {
-      'app/stewards.yml'        => [Ladle::StewardChangesView.new(
-                                      stewards_file: 'app/stewards.yml',
-                                      changes:       [
-                                                       build(:file_change, status: :removed, file: "app/removed_file.rb", deletions: 6),
-                                                       build(:file_change, status: :modified, file: "app/modified_file.rb", deletions: 3, additions: 3),
-                                                       build(:file_change, status: :added, file: "app/new_file.rb", additions: 6),
-                                                     ])],
-      'lib/closet/stewards.yml' => [Ladle::StewardChangesView.new(
-                                      stewards_file: 'lib/closet/stewards.yml',
-                                      changes:       [
-                                                       build(:file_change, status: :added, file: "lib/closet/top_shelf/new_file.rb", additions: 6),
-                                                     ])],
-    }
   end
 end
diff --git a/test/mailers/user_mailer_test.rb b/test/mailers/user_mailer_test.rb
index d4ea01c..5339ab7 100644
--- a/test/mailers/user_mailer_test.rb
+++ b/test/mailers/user_mailer_test.rb
@@ -1,4 +1,5 @@
 require 'test_helper'
+require 'ladle/test_data'
 
 class UserMailerTest < ActionMailer::TestCase
 
@@ -13,7 +14,7 @@ class UserMailerTest < ActionMailer::TestCase
                                 nil,
                                 nil
                               ),
-                              stewards_file_map: create_steward_changes_views).deliver_now
+                              stewards_file_map: Ladle::TestData.create_steward_changes_views).deliver_now
 
     assert_not ActionMailer::Base.deliveries.empty?
 
@@ -32,7 +33,7 @@ class UserMailerTest < ActionMailer::TestCase
                                 'Hey ho!',
                                 'These changes are luminous',
                               ),
-                              stewards_file_map: create_steward_changes_views).deliver_now
+                              stewards_file_map: Ladle::TestData.create_steward_changes_views).deliver_now
 
     assert_not ActionMailer::Base.deliveries.empty?
 
@@ -41,23 +42,4 @@ class UserMailerTest < ActionMailer::TestCase
     assert_equal [user.email], email.to
     assert_equal '[XanderStrike/test] Ladle Alert: Hey ho!', email.subject
   end
-
-  private
-
-  def create_steward_changes_views
-    {
-      'app/stewards.yml'        => [Ladle::StewardChangesView.new(
-                                      stewards_file: 'app/stewards.yml',
-                                      changes:       [
-                                                       build(:file_change, status: :removed, file: "app/removed_file.rb", deletions: 6),
-                                                       build(:file_change, status: :modified, file: "app/modified_file.rb", deletions: 3, additions: 3),
-                                                       build(:file_change, status: :added, file: "app/new_file.rb", additions: 6),
-                                                     ])],
-      'lib/closet/stewards.yml' => [Ladle::StewardChangesView.new(
-                                      stewards_file: 'lib/closet/stewards.yml',
-                                      changes:       [
-                                                       build(:file_change, status: :added, file: "lib/closet/top_shelf/new_file.rb", additions: 6),
-                                                     ])],
-    }
-  end
 end

From 1350710ba195478bdf7c7325ee354de73317c55a Mon Sep 17 00:00:00 2001
From: Donnie Tognazzini <donnie.tognazzini@appfolio.com>
Date: Mon, 14 Dec 2015 15:05:53 -0800
Subject: [PATCH 09/15] Removing unnecessary .keep files

---
 app/assets/images/.keep | 0
 app/mailers/.keep       | 0
 lib/assets/.keep        | 0
 lib/tasks/.keep         | 0
 test/helpers/.keep      | 0
 test/integration/.keep  | 0
 test/mailers/.keep      | 0
 7 files changed, 0 insertions(+), 0 deletions(-)
 delete mode 100644 app/assets/images/.keep
 delete mode 100644 app/mailers/.keep
 delete mode 100644 lib/assets/.keep
 delete mode 100644 lib/tasks/.keep
 delete mode 100644 test/helpers/.keep
 delete mode 100644 test/integration/.keep
 delete mode 100644 test/mailers/.keep

diff --git a/app/assets/images/.keep b/app/assets/images/.keep
deleted file mode 100644
index e69de29..0000000
diff --git a/app/mailers/.keep b/app/mailers/.keep
deleted file mode 100644
index e69de29..0000000
diff --git a/lib/assets/.keep b/lib/assets/.keep
deleted file mode 100644
index e69de29..0000000
diff --git a/lib/tasks/.keep b/lib/tasks/.keep
deleted file mode 100644
index e69de29..0000000
diff --git a/test/helpers/.keep b/test/helpers/.keep
deleted file mode 100644
index e69de29..0000000
diff --git a/test/integration/.keep b/test/integration/.keep
deleted file mode 100644
index e69de29..0000000
diff --git a/test/mailers/.keep b/test/mailers/.keep
deleted file mode 100644
index e69de29..0000000

From bac3786af9ee9ba55c61fa1324baa1d56987d92d Mon Sep 17 00:00:00 2001
From: Donnie Tognazzini <donnie.tognazzini@appfolio.com>
Date: Mon, 14 Dec 2015 15:30:47 -0800
Subject: [PATCH 10/15] Adding StewardChangesView#ref and including ref in
 notification email.

---
 app/views/user_mailer/notify.html.erb        |  2 +-
 app/views/user_mailer/notify.text.erb        |  2 +-
 lib/ladle/pull_handler.rb                    |  3 +-
 lib/ladle/steward_changes_view.rb            | 10 ++-
 lib/ladle/test_data.rb                       | 35 ++++----
 test/factories/ladle/steward_changes_view.rb |  4 +-
 test/ladle/pull_handler_test.rb              | 84 ++++++++++++--------
 7 files changed, 83 insertions(+), 57 deletions(-)

diff --git a/app/views/user_mailer/notify.html.erb b/app/views/user_mailer/notify.html.erb
index 43aa0d0..bb4b2a8 100644
--- a/app/views/user_mailer/notify.html.erb
+++ b/app/views/user_mailer/notify.html.erb
@@ -12,7 +12,7 @@
   <% @stewards_file_map.each do |stewards_file_path, steward_change_views| %>
     <% steward_change_views.each do |steward_change_view| %>
   <li>
-    <p>Changes monitored by <%= steward_change_view.stewards_file %>:</p>
+    <p>Changes monitored by <%= steward_change_view.ref %>/<%= steward_change_view.stewards_file %>:</p>
     <ul>
       <% steward_change_view.changes.each do |change| %>
         <li><strong><%= change.status_initial %></strong> <%= change.file %> (<%= change.changes_count %>)</li>
diff --git a/app/views/user_mailer/notify.text.erb b/app/views/user_mailer/notify.text.erb
index b35c021..dd70846 100644
--- a/app/views/user_mailer/notify.text.erb
+++ b/app/views/user_mailer/notify.text.erb
@@ -12,7 +12,7 @@ You were notified of this pull request because of the following changes:
 
 <% @stewards_file_map.each do |stewards_file_path, steward_change_views| %>
   <% steward_change_views.each do |steward_change_view| %>
-  Changes monitored by <%= steward_change_view.stewards_file %>:
+  Changes monitored by <%= steward_change_view.ref %>/<%= steward_change_view.stewards_file %>:
 
   <% steward_change_view.changes.each do |change| %>
     * <%= change.status_initial %> <%= change.file %> (<%= change.changes_count %>)
diff --git a/lib/ladle/pull_handler.rb b/lib/ladle/pull_handler.rb
index d22d53b..a5cae1a 100644
--- a/lib/ladle/pull_handler.rb
+++ b/lib/ladle/pull_handler.rb
@@ -53,7 +53,8 @@ def register_stewards(stewards_user_tree_map, stewards_file_path, sha)
       stewards_file = StewardsFileParser.parse(contents)
 
       stewards_file.stewards.each do |steward_config|
-        changes_view = Ladle::StewardChangesView.new(stewards_file: stewards_file_path,
+        changes_view = Ladle::StewardChangesView.new(ref: sha,
+                                                     stewards_file: stewards_file_path,
                                                      file_filter: steward_config.file_filter)
 
         stewards_user_tree_map[steward_config.github_username] ||= {}
diff --git a/lib/ladle/steward_changes_view.rb b/lib/ladle/steward_changes_view.rb
index a51aff4..4228de5 100644
--- a/lib/ladle/steward_changes_view.rb
+++ b/lib/ladle/steward_changes_view.rb
@@ -2,9 +2,10 @@
 
 module Ladle
   class StewardChangesView
-    attr_reader :stewards_file, :file_filter, :changes
+    attr_reader :ref, :stewards_file, :file_filter, :changes
 
-    def initialize(stewards_file:, file_filter: nil, changes: nil)
+    def initialize(ref:, stewards_file:, file_filter: nil, changes: nil)
+      @ref           = ref
       @file_filter   = file_filter || FileFilter.new
       @stewards_file = Pathname.new(stewards_file)
       @changes       = changes || []
@@ -23,13 +24,14 @@ def add_file_changes(file_changes)
     end
 
     def ==(other)
-      @stewards_file.to_s == other.stewards_file.to_s &&
+      @ref == other.ref &&
+        @stewards_file.to_s == other.stewards_file.to_s &&
         @file_filter == other.file_filter &&
         @changes.to_set == other.changes.to_set
     end
 
     def hash
-      [@stewards_file, @file_filter, @changes].hash
+      [@ref, @stewards_file, @file_filter, @changes].hash
     end
 
     alias eql? ==
diff --git a/lib/ladle/test_data.rb b/lib/ladle/test_data.rb
index eed92ba..00f83f1 100644
--- a/lib/ladle/test_data.rb
+++ b/lib/ladle/test_data.rb
@@ -4,23 +4,24 @@ module TestData
 
     def self.create_steward_changes_views
       {
-        'app/stewards.yml'        => [Ladle::StewardChangesView.new(
-                                        stewards_file: 'app/stewards.yml',
-                                        changes:       [
-                                                         build(:file_change, status: :removed, file: "app/removed_file.rb", deletions: 6),
-                                                         build(:file_change, status: :modified, file: "app/modified_file.rb", deletions: 3, additions: 3),
-                                                         build(:file_change, status: :added, file: "app/new_file.rb", additions: 6),
-                                                       ]),
-                                      Ladle::StewardChangesView.new(
-                                        stewards_file: 'app/stewards.yml',
-                                        changes:       [
-                                                         build(:file_change, status: :modified, file: "app/blah.rb", deletions: 3, additions: 3),
-                                                       ])],
-        'lib/closet/stewards.yml' => [Ladle::StewardChangesView.new(
-                                        stewards_file: 'lib/closet/stewards.yml',
-                                        changes:       [
-                                                         build(:file_change, status: :added, file: "lib/closet/top_shelf/new_file.rb", additions: 6),
-                                                       ])],
+        'app/stewards.yml'        => [build(:steward_changes_view,
+                                            stewards_file: 'app/stewards.yml',
+                                            changes:       [
+                                                             build(:file_change, status: :removed, file: "app/removed_file.rb", deletions: 6),
+                                                             build(:file_change, status: :modified, file: "app/modified_file.rb", deletions: 3, additions: 3),
+                                                             build(:file_change, status: :added, file: "app/new_file.rb", additions: 6),
+                                                           ]),
+                                      build(:steward_changes_view,
+                                            ref:           'branch',
+                                            stewards_file: 'app/stewards.yml',
+                                            changes:       [
+                                                             build(:file_change, status: :modified, file: "app/blah.rb", deletions: 3, additions: 3),
+                                                           ])],
+        'lib/closet/stewards.yml' => [build(:steward_changes_view,
+                                            stewards_file: 'lib/closet/stewards.yml',
+                                            changes:       [
+                                                             build(:file_change, status: :added, file: "lib/closet/top_shelf/new_file.rb", additions: 6),
+                                                           ])],
       }
     end
   end
diff --git a/test/factories/ladle/steward_changes_view.rb b/test/factories/ladle/steward_changes_view.rb
index 4dad30e..ad29ebd 100644
--- a/test/factories/ladle/steward_changes_view.rb
+++ b/test/factories/ladle/steward_changes_view.rb
@@ -1,11 +1,13 @@
 FactoryGirl.define do
   factory :steward_changes_view, class: "Ladle::StewardChangesView" do
+    ref 'base'
     stewards_file 'stewards.yml'
     file_filter nil
     changes nil
 
     initialize_with do
-      new(stewards_file: stewards_file,
+      new(ref:           ref,
+          stewards_file: stewards_file,
           file_filter:   file_filter,
           changes:       changes)
     end
diff --git a/test/ladle/pull_handler_test.rb b/test/ladle/pull_handler_test.rb
index a9a4611..3a56ddc 100644
--- a/test/ladle/pull_handler_test.rb
+++ b/test/ladle/pull_handler_test.rb
@@ -52,6 +52,7 @@ class PullHandlerTest < ActiveSupport::TestCase
     YAML
 
     expected_stewards_changes_view = build(:steward_changes_view,
+                                           ref: 'base_head',
                                            stewards_file: 'stewards.yml',
                                            changes:       [
                                                             build(:file_change, status: :added, file: 'one.rb', additions: 1),
@@ -59,6 +60,7 @@ class PullHandlerTest < ActiveSupport::TestCase
                                                           ])
 
     expected_sub_stewards_changes_view = build(:steward_changes_view,
+                                               ref: 'base_head',
                                                stewards_file: 'sub/stewards.yml',
                                                changes:       [
                                                                 build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1, deletions: 1),
@@ -75,6 +77,7 @@ class PullHandlerTest < ActiveSupport::TestCase
               },
               'bob'               => {
                 'stewards.yml' => [build(:steward_changes_view,
+                                         ref: 'base_head',
                                          stewards_file: 'stewards.yml',
                                          file_filter:   Ladle::FileFilter.new(
                                            include_patterns: ["**.rb"],
@@ -112,6 +115,7 @@ class PullHandlerTest < ActiveSupport::TestCase
     YAML
 
     base_changes = build(:steward_changes_view,
+                         ref: 'base_head',
                          stewards_file: 'stewards.yml',
                          file_filter:   Ladle::FileFilter.new(
                            include_patterns: ["file1.txt"]
@@ -121,6 +125,7 @@ class PullHandlerTest < ActiveSupport::TestCase
                                         ])
 
     branch_changes = build(:steward_changes_view,
+                           ref: 'branch_head',
                            stewards_file: 'stewards.yml',
                            file_filter:   Ladle::FileFilter.new(
                              include_patterns: ["file2.txt"]
@@ -163,6 +168,7 @@ class PullHandlerTest < ActiveSupport::TestCase
     YAML
 
     changes = build(:steward_changes_view,
+                    ref: 'base_head',
                     stewards_file: 'stewards.yml',
                     file_filter:   Ladle::FileFilter.new(
                       include_patterns: ["file1.txt"]
@@ -226,6 +232,7 @@ class PullHandlerTest < ActiveSupport::TestCase
     YAML
 
     expected_stewards_changes_view = build(:steward_changes_view,
+                                           ref: 'base_head',
                                            stewards_file: 'stewards.yml',
                                            changes:       [
                                                             build(:file_change, status: :removed, file: 'stewards.yml', deletions: 1),
@@ -238,6 +245,7 @@ class PullHandlerTest < ActiveSupport::TestCase
                                                           ])
 
     expected_sub_stewards_changes_view = build(:steward_changes_view,
+                                               ref:           'branch_head',
                                                stewards_file: 'sub/stewards.yml',
                                                changes:       [
                                                                 build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1),
@@ -245,13 +253,15 @@ class PullHandlerTest < ActiveSupport::TestCase
                                                               ])
 
     expected_sub2_stewards_changes_view = build(:steward_changes_view,
-                                               stewards_file: 'sub2/stewards.yml',
-                                               changes:       [
-                                                                build(:file_change, status: :removed, file: 'sub2/sandwich', deletions: 1),
-                                                                build(:file_change, status: :added, file: 'sub2/stewards.yml', additions: 1)
-                                                              ])
+                                                ref:           'branch_head',
+                                                stewards_file: 'sub2/stewards.yml',
+                                                changes:       [
+                                                                 build(:file_change, status: :removed, file: 'sub2/sandwich', deletions: 1),
+                                                                 build(:file_change, status: :added, file: 'sub2/stewards.yml', additions: 1)
+                                                               ])
 
     expected_sub3_stewards_changes_view = build(:steward_changes_view,
+                                                ref:           'base_head',
                                                 stewards_file: 'sub3/stewards.yml',
                                                 changes:       [
                                                                  build(:file_change, status: :removed, file: 'sub3/stewards.yml', deletions: 1)
@@ -273,7 +283,13 @@ class PullHandlerTest < ActiveSupport::TestCase
                 'sub3/stewards.yml' => [expected_sub3_stewards_changes_view]
               },
               'jeb'               => {
-                'sub/stewards.yml' => [expected_sub_stewards_changes_view]
+                'sub/stewards.yml' => [build(:steward_changes_view,
+                                             ref:           'base_head',
+                                             stewards_file: 'sub/stewards.yml',
+                                             changes:       [
+                                                              build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1),
+                                                              build(:file_change, status: :modified, file: 'sub/stewards.yml', additions: 1)
+                                                            ])]
               },
               'hamburglar'        => {
                 'sub2/stewards.yml' => [expected_sub2_stewards_changes_view]
@@ -303,17 +319,19 @@ class PullHandlerTest < ActiveSupport::TestCase
     YAML
 
     expected_stewards_changes_view = build(:steward_changes_view,
-                                    stewards_file: 'hello/stewards.yml',
-                                    changes:       [
-                                                     build(:file_change, status: :added, file: 'hello/kitty/what/che.txt', additions: 1),
-                                                     build(:file_change, status: :removed, file: 'hello/kitty/what/is/stewards.yml', deletions: 1),
-                                                   ])
+                                           ref:           'base_head',
+                                           stewards_file: 'hello/stewards.yml',
+                                           changes:       [
+                                                            build(:file_change, status: :added, file: 'hello/kitty/what/che.txt', additions: 1),
+                                                            build(:file_change, status: :removed, file: 'hello/kitty/what/is/stewards.yml', deletions: 1),
+                                                          ])
 
     expected_sub_stewards_changes_view = build(:steward_changes_view,
-                                                  stewards_file: 'hello/kitty/what/is/stewards.yml',
-                                                  changes:       [
-                                                                   build(:file_change, status: :removed, file: 'hello/kitty/what/is/stewards.yml', deletions: 1),
-                                                                 ])
+                                               ref:           'base_head',
+                                               stewards_file: 'hello/kitty/what/is/stewards.yml',
+                                               changes:       [
+                                                                build(:file_change, status: :removed, file: 'hello/kitty/what/is/stewards.yml', deletions: 1),
+                                                              ])
 
     notifier = mock
     notifier.expects(:notify)
@@ -349,11 +367,12 @@ class PullHandlerTest < ActiveSupport::TestCase
 
     expected_stewards_changes_view = {
       'stewards.yml' => [build(:steward_changes_view,
-                              stewards_file: 'stewards.yml',
-                              changes:       [
-                                               build(:file_change, status: :added, file: 'one.rb', additions: 1),
-                                               build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1, deletions: 1),
-                                             ])]
+                               ref:           'base_head',
+                               stewards_file: 'stewards.yml',
+                               changes:       [
+                                                build(:file_change, status: :added, file: 'one.rb', additions: 1),
+                                                build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1, deletions: 1),
+                                              ])]
     }
 
     notifier = mock
@@ -386,15 +405,16 @@ class PullHandlerTest < ActiveSupport::TestCase
     notifier.stubs(:id).returns(1)
     notifier.expects(:notify)
       .with(deep_hash({
-              'xanderstrike' => {
-                'hello/stewards.yml' => [build(:steward_changes_view,
-                                              stewards_file: 'hello/stewards.yml',
-                                              changes:       [
-                                                               build(:file_change, status: :added, file: 'hello/kitty/what/is/your/favorite_food.yml', additions: 1),
-                                                               build(:file_change, status: :added, file: 'hello/kitty/what/is/your/name.txt', additions: 1)
-                                                             ])],
-              },
-            }))
+                        'xanderstrike' => {
+                          'hello/stewards.yml' => [build(:steward_changes_view,
+                                                         ref:           'base_head',
+                                                         stewards_file: 'hello/stewards.yml',
+                                                         changes:       [
+                                                                          build(:file_change, status: :added, file: 'hello/kitty/what/is/your/favorite_food.yml', additions: 1),
+                                                                          build(:file_change, status: :added, file: 'hello/kitty/what/is/your/name.txt', additions: 1)
+                                                                        ])],
+                        },
+                      }))
 
     Ladle::PullHandler.new(client, notifier).handle(@pull_request)
   end
@@ -419,7 +439,7 @@ class PullHandlerTest < ActiveSupport::TestCase
     handler = Ladle::PullHandler.new(mock('client'), mock('notifier'))
     resolved_stewards_registry = handler.send(:resolve_stewards_scope, registry, changed_files)
 
-    expected_changes_view = Ladle::StewardChangesView.new(
+    expected_changes_view = build(:steward_changes_view,
       stewards_file: 'stewards.yml',
       changes:       [
                        build(:file_change, file: 'stewards.yml'),
@@ -432,7 +452,7 @@ class PullHandlerTest < ActiveSupport::TestCase
 
     assert_equal [expected_changes_view], resolved_stewards_registry['xanderstrike']['stewards.yml']
 
-    expected_changes_view = Ladle::StewardChangesView.new(
+    expected_changes_view = build(:steward_changes_view,
       stewards_file: 'sub/stewards.yml',
       changes:       [
                        build(:file_change, file: 'sub/marine.rb'),
@@ -441,7 +461,7 @@ class PullHandlerTest < ActiveSupport::TestCase
 
     assert_equal [expected_changes_view], resolved_stewards_registry['xanderstrike']['sub/stewards.yml']
 
-    expected_changes_view = Ladle::StewardChangesView.new(
+    expected_changes_view = build(:steward_changes_view,
       stewards_file: 'sub3/stewards.yml',
       changes:       [
                        build(:file_change, file: 'sub3/stewards.yml')

From c6d14494f51f4cb4060bedda3dcd043f2c8cf899 Mon Sep 17 00:00:00 2001
From: Donnie Tognazzini <donnie.tognazzini@appfolio.com>
Date: Mon, 14 Dec 2015 15:41:38 -0800
Subject: [PATCH 11/15] Adding a tree definition DSL to simplify test setup

---
 test/ladle/pull_handler_test.rb   | 132 ++++++++++++++++--------------
 test/ladle/stubbed_repo_client.rb |  15 ++++
 2 files changed, 85 insertions(+), 62 deletions(-)

diff --git a/test/ladle/pull_handler_test.rb b/test/ladle/pull_handler_test.rb
index 3a56ddc..dae0848 100644
--- a/test/ladle/pull_handler_test.rb
+++ b/test/ladle/pull_handler_test.rb
@@ -37,19 +37,22 @@ class PullHandlerTest < ActiveSupport::TestCase
 
     client = Ladle::StubbedRepoClient.new(@pull_request.number, Ladle::PullRequestInfo.new('branch_head', 'base_head'), changed_files)
 
-    client.add_stewards_file(path: 'sub/stewards.yml', ref: 'base_head', contents: <<-YAML)
-      stewards:
-        - xanderstrike
-        - fadsfadsfadsfadsf
-    YAML
-
-    client.add_stewards_file(path: 'stewards.yml', ref: 'base_head', contents: <<-YAML)
-      stewards:
-        - github_username: bob
-          include: "**.rb"
-          exclude: "**.txt"
-        - xanderstrike
-    YAML
+    client.define_tree('base_head') do |tree|
+
+      tree.file('sub/stewards.yml', <<-YAML)
+        stewards:
+          - xanderstrike
+          - fadsfadsfadsfadsf
+      YAML
+
+      tree.file('stewards.yml', <<-YAML)
+        stewards:
+          - github_username: bob
+            include: "**.rb"
+            exclude: "**.txt"
+          - xanderstrike
+      YAML
+    end
 
     expected_stewards_changes_view = build(:steward_changes_view,
                                            ref: 'base_head',
@@ -199,37 +202,37 @@ class PullHandlerTest < ActiveSupport::TestCase
 
     client = Ladle::StubbedRepoClient.new(@pull_request.number, Ladle::PullRequestInfo.new('branch_head', 'base_head'), changed_files)
 
-    # sub3/stewards.yml removed by branch
-    client.add_stewards_file(path: 'sub3/stewards.yml', ref: 'base_head', contents: <<-YAML)
-      stewards:
-        - xanderstrike
-        - bob
-    YAML
-
-    # sub2/stewards.yml added by branch
-    client.add_stewards_file(path: 'sub2/stewards.yml', ref: 'branch_head', contents: <<-YAML)
-      stewards:
-        - hamburglar
-    YAML
-
-    # sub/stewards.yml exists on both, changed by branch
-    client.add_stewards_file(path: 'sub/stewards.yml', ref: 'base_head', contents: <<-YAML)
-      stewards:
-        - jeb
-    YAML
-
-    client.add_stewards_file(path: 'sub/stewards.yml', ref: 'branch_head', contents: <<-YAML)
-      stewards:
-        - xanderstrike
-        - fadsfadsfadsfadsf
-    YAML
-
-    # stewards.yml exists on base
-    client.add_stewards_file(path: 'stewards.yml', ref: 'base_head', contents: <<-YAML)
-      stewards:
-        - xanderstrike
-        - bob
-    YAML
+    client.define_tree('base_head') do |tree|
+      tree.file('sub3/stewards.yml', <<-YAML)
+        stewards:
+          - xanderstrike
+          - bob
+      YAML
+
+      tree.file('sub/stewards.yml', <<-YAML)
+        stewards:
+          - jeb
+      YAML
+
+      tree.file('stewards.yml', <<-YAML)
+        stewards:
+          - xanderstrike
+          - bob
+      YAML
+    end
+
+    client.define_tree('branch_head') do |tree|
+      tree.file('sub2/stewards.yml', <<-YAML)
+        stewards:
+          - hamburglar
+      YAML
+
+      tree.file('sub/stewards.yml', <<-YAML)
+        stewards:
+          - xanderstrike
+          - fadsfadsfadsfadsf
+      YAML
+    end
 
     expected_stewards_changes_view = build(:steward_changes_view,
                                            ref: 'base_head',
@@ -307,16 +310,19 @@ class PullHandlerTest < ActiveSupport::TestCase
 
     client = Ladle::StubbedRepoClient.new(@pull_request.number, Ladle::PullRequestInfo.new('branch_head', 'base_head'), changed_files)
 
-    client.add_stewards_file(path: 'hello/kitty/what/is/stewards.yml', ref: 'base_head', contents: <<-YAML)
-      stewards:
-        - xanderstrike
-        - bleh
-    YAML
+    client.define_tree('base_head') do |tree|
 
-    client.add_stewards_file(path: 'hello/stewards.yml', ref: 'base_head', contents: <<-YAML)
-      stewards:
-        - xanderstrike
-    YAML
+      tree.file('hello/kitty/what/is/stewards.yml', <<-YAML)
+        stewards:
+          - xanderstrike
+          - bleh
+      YAML
+
+      tree.file('hello/stewards.yml', <<-YAML)
+        stewards:
+          - xanderstrike
+      YAML
+    end
 
     expected_stewards_changes_view = build(:steward_changes_view,
                                            ref:           'base_head',
@@ -355,15 +361,17 @@ class PullHandlerTest < ActiveSupport::TestCase
 
     client = Ladle::StubbedRepoClient.new(@pull_request.number, Ladle::PullRequestInfo.new('branch_head', 'base_head'), changed_files)
 
-    client.add_stewards_file(path: 'sub/stewards.yml', ref: 'base_head', contents: <<-YAML)
-      SOME WORDS
-    YAML
-
-    client.add_stewards_file(path: 'stewards.yml', ref: 'base_head', contents: <<-YAML)
-      stewards:
-        - xanderstrike
-        - fadsfadsfadsfadsf
-    YAML
+    client.define_tree('base_head') do |tree|
+      tree.file('sub/stewards.yml', <<-YAML)
+        SOME WORDS
+      YAML
+
+      tree.file('stewards.yml', <<-YAML)
+        stewards:
+          - xanderstrike
+          - fadsfadsfadsfadsf
+      YAML
+    end
 
     expected_stewards_changes_view = {
       'stewards.yml' => [build(:steward_changes_view,
diff --git a/test/ladle/stubbed_repo_client.rb b/test/ladle/stubbed_repo_client.rb
index 2bec36d..39c031d 100644
--- a/test/ladle/stubbed_repo_client.rb
+++ b/test/ladle/stubbed_repo_client.rb
@@ -1,5 +1,16 @@
 module Ladle
   class StubbedRepoClient
+    class TreeDefinition
+      def initialize(ref, repo)
+        @ref = ref
+        @repo = repo
+      end
+
+      def file(path, contents)
+        @repo.add_stewards_file(ref: @ref, path: path, contents: contents)
+      end
+    end
+
     def initialize(pull_request_number, pull_request_info, changed_files)
       @pull_request_number = pull_request_number
       @pull_request_info   = pull_request_info
@@ -17,6 +28,10 @@ def add_stewards_file(ref:, path:, contents:)
       repo_at_ref[path] = contents
     end
 
+    def define_tree(ref)
+      yield TreeDefinition.new(ref, self)
+    end
+
     def contents(path:, ref:)
       repo_at_ref = @repo[ref]
       raise "Ref #{ref.inspect} not in repo" unless repo_at_ref

From 45af788f0b6808c677ab3ea0a146e1304e74f769 Mon Sep 17 00:00:00 2001
From: Donnie Tognazzini <donnie.tognazzini@appfolio.com>
Date: Tue, 15 Dec 2015 10:01:50 -0800
Subject: [PATCH 12/15] Layering in some objects

---
 lib/ladle.rb                            |   1 +
 lib/ladle/pull_handler.rb               | 161 +++++++++++++-----------
 lib/ladle/steward_changes_view.rb       |  12 --
 lib/ladle/steward_rules.rb              |  31 +++++
 test/factories/ladle/steward_rules.rb   |  13 ++
 test/ladle/pull_handler_test.rb         |  27 ++--
 test/ladle/steward_changes_view_test.rb |  22 ----
 test/ladle/steward_rules_test.rb        |  15 +++
 8 files changed, 167 insertions(+), 115 deletions(-)
 create mode 100644 lib/ladle/steward_rules.rb
 create mode 100644 test/factories/ladle/steward_rules.rb
 create mode 100644 test/ladle/steward_rules_test.rb

diff --git a/lib/ladle.rb b/lib/ladle.rb
index d15074f..cd7aedf 100644
--- a/lib/ladle.rb
+++ b/lib/ladle.rb
@@ -3,6 +3,7 @@ module Ladle
 
   autoload :FileChange
   autoload :StewardChangesView
+  autoload :StewardRules
 end
 
 require 'ladle/exceptions'
diff --git a/lib/ladle/pull_handler.rb b/lib/ladle/pull_handler.rb
index a5cae1a..8d2b0b1 100644
--- a/lib/ladle/pull_handler.rb
+++ b/lib/ladle/pull_handler.rb
@@ -1,5 +1,5 @@
 require 'ladle/stewards_file_parser'
-require 'ladle/steward_changes_view'
+require 'ladle/steward_rules'
 
 module Ladle
   class PullHandler
@@ -11,15 +11,11 @@ def initialize(client, notifier)
     def handle(pull_request)
       pr_info = @client.pull_request(pull_request.number)
 
-      pull_request_files = @client.pull_request_files(pull_request.number)
+      pr_files = @client.pull_request_files(pull_request.number)
 
-      stewards_from_base = read_stewards(pull_request_files, pr_info.base_sha)
-      stewards_from_base = resolve_stewards_scope(stewards_from_base, pull_request_files)
+      stewards_trees = collect_stewards_rules(pull_request, pr_info, pr_files)
 
-      stewards_from_head = read_new_stewards(pull_request_files, pr_info.head_sha)
-      stewards_from_head = resolve_stewards_scope(stewards_from_head, pull_request_files)
-
-      stewards_registry = combine_stewards_tree_maps(stewards_from_base, stewards_from_head)
+      stewards_registry = collect_changes(stewards_trees, pr_files)
 
       if stewards_registry.empty?
         Rails.logger.info('No stewards found. Doing nothing.')
@@ -31,103 +27,124 @@ def handle(pull_request)
 
     private
 
-    def read_new_stewards(pull_request_files, base_head)
-      stewards_user_tree_map = {} #StewardsUserTreeMap.new
-      pull_request_files.modified_stewards_files.each do |stewards_file_path|
-        register_stewards(stewards_user_tree_map, stewards_file_path, base_head)
+    def collect_stewards_rules(pull_request, pr_info, pr_files)
+      rules = {}
+
+      pr_files.directories.each do |directory|
+        register_stewards(rules, directory.join('stewards.yml'), pr_info.base_sha)
       end
-      stewards_user_tree_map
-    end
 
-    def read_stewards(pull_request_files, pr_head)
-      stewards_user_tree_map = {} #StewardsUserTreeMap.new
-      pull_request_files.directories.each do |directory|
-        register_stewards(stewards_user_tree_map, directory.join('stewards.yml'), pr_head)
+      pr_files.modified_stewards_files.each do |stewards_file_path|
+        register_stewards(rules, stewards_file_path, pr_info.head_sha)
       end
 
-      stewards_user_tree_map
+      rules
     end
 
-    def register_stewards(stewards_user_tree_map, stewards_file_path, sha)
-      contents = @client.contents(path: stewards_file_path.to_s, ref: sha)
-      stewards_file = StewardsFileParser.parse(contents)
+    class ChangesView
+      attr_reader :paths
 
-      stewards_file.stewards.each do |steward_config|
-        changes_view = Ladle::StewardChangesView.new(ref: sha,
-                                                     stewards_file: stewards_file_path,
-                                                     file_filter: steward_config.file_filter)
+      RulesChanges = Struct.new(:rules, :changes)
 
-        stewards_user_tree_map[steward_config.github_username] ||= {}
-        stewards_user_tree_map[steward_config.github_username][stewards_file_path.to_s] = [changes_view]
+      def initialize
+        @paths = {}
       end
-    rescue Ladle::RemoteFileNotFound
-      # Ignore - stewards files don't have to exist
-    rescue StewardsFileParser::ParsingError => e
-      Rails.logger.error("Error parsing file #{stewards_file_path}: #{e.message}\n#{e.backtrace.join("\n")}")
-    end
 
-    def select_non_empty_change_views(steward_tree, pull_request_files)
-      steward_tree.reject do |stewards_file_path, change_views|
-        change_view = change_views.first
-        file_changes = pull_request_files.file_changes_in(change_view.stewards_file.dirname)
-        change_view.add_file_changes(file_changes)
-        change_view.empty?
+      def empty?
+        @paths.values.all?(&:empty?)
       end
-    end
 
-    def resolve_stewards_scope(stewards_user_tree_map, pull_request_files)
-      output = {}
+      def add_changes(rules, changes)
+        @paths[rules.stewards_file.to_s] ||= []
+
+        add_changes_to(@paths[rules.stewards_file.to_s], rules, changes)
+      end
 
-      stewards_user_tree_map.each do |github_username, steward_tree|
-        non_empty_file_map = select_non_empty_change_views(steward_tree, pull_request_files)
+      private
 
-        unless non_empty_file_map.empty?
-          output[github_username] = non_empty_file_map
+      def add_changes_to(rules_and_changes_collection, rules, changes)
+        changes_already_exist = rules_and_changes_collection.any? do |rules_changes|
+          rules_changes.changes == changes
         end
-      end
 
-      output
+        unless changes_already_exist
+          rules_and_changes_collection << RulesChanges.new(rules, changes)
+        end
+      end
     end
 
-    def combine_stewards_tree_maps(left_steward_tree_map, right_steward_tree_map)
-      combined = {}
+    class StewardTree
+      attr_reader :github_username
 
-      github_usernames = (left_steward_tree_map.keys + right_steward_tree_map.keys).uniq
-      github_usernames.each do |github_username|
-        left_steward_tree  = left_steward_tree_map[github_username]
-        right_steward_tree = right_steward_tree_map[github_username]
+      def initialize(github_username)
+        @github_username = github_username
+        @rules           = []
+      end
 
-        combined[github_username] = combine_steward_trees(left_steward_tree, right_steward_tree)
+      def add_rules(rules)
+        @rules << rules
       end
 
-      combined
+      def changes(pull_request_files)
+        changes_view = ChangesView.new
+
+        @rules.each do |rules|
+          file_changes = pull_request_files.file_changes_in(rules.stewards_file.dirname)
+          file_changes = rules.select_matching_file_changes(file_changes)
+
+          unless file_changes.empty?
+            changes_view.add_changes(rules, file_changes)
+          end
+        end
+
+        changes_view
+      end
     end
 
-    def combine_steward_trees(left_steward_tree, right_steward_tree)
-      if left_steward_tree.nil? || right_steward_tree.nil?
-        return right_steward_tree.presence || left_steward_tree.presence
+    def register_stewards(stewards_rules_map, stewards_file_path, sha)
+      contents = @client.contents(path: stewards_file_path.to_s, ref: sha)
+      stewards_file = StewardsFileParser.parse(contents)
+
+      stewards_file.stewards.each do |steward_config|
+        rules = Ladle::StewardRules.new(ref:           sha,
+                                        stewards_file: stewards_file_path,
+                                        file_filter:   steward_config.file_filter)
+
+        stewards_rules_map[steward_config.github_username] ||= StewardTree.new(steward_config.github_username)
+        stewards_rules_map[steward_config.github_username].add_rules(rules)
       end
+    rescue Ladle::RemoteFileNotFound
+      # Ignore - stewards files don't have to exist
+    rescue StewardsFileParser::ParsingError => e
+      Rails.logger.error("Error parsing file #{stewards_file_path}: #{e.message}\n#{e.backtrace.join("\n")}")
+    end
 
-      combined = {}
+    def collect_changes(stewards_trees, pull_request_files)
+      output = {}
 
-      all_stewards_file_paths = (left_steward_tree.keys + right_steward_tree.keys).uniq
-      all_stewards_file_paths.each do |stewards_file_path|
-        left_change_views  = left_steward_tree[stewards_file_path]
-        right_change_views = right_steward_tree[stewards_file_path]
+      stewards_trees.each do |github_username, steward_tree|
+        changes_view = steward_tree.changes(pull_request_files)
 
-        left_change_views = left_change_views.first unless left_change_views.nil?
-        right_change_views = right_change_views.first unless right_change_views.nil?
+        unless changes_view.empty?
+          output[github_username] = map_to_old_stewards_tree_change_view(changes_view)
+        end
+      end
 
-        all_changes = [left_change_views, right_change_views].reject(&:nil?)
+      output
+    end
 
-        if all_changes.size == 2 && all_changes.first.changes == all_changes.last.changes
-          combined[stewards_file_path] = [all_changes.first]
-        else
-          combined[stewards_file_path] = all_changes
+    def map_to_old_stewards_tree_change_view(changes_view)
+      old_datastructure = {}
+      changes_view.paths.each do |stewards_file_path, rules_with_changes_list|
+        old_datastructure[stewards_file_path] = rules_with_changes_list.map do |rule_with_changes|
+          Ladle::StewardChangesView.new(ref:           rule_with_changes.rules.ref,
+                                        stewards_file: rule_with_changes.rules.stewards_file,
+                                        file_filter:   rule_with_changes.rules.file_filter,
+                                        changes:       rule_with_changes.changes)
         end
       end
 
-      combined
+      old_datastructure
     end
   end
 end
diff --git a/lib/ladle/steward_changes_view.rb b/lib/ladle/steward_changes_view.rb
index 4228de5..fe9f2cc 100644
--- a/lib/ladle/steward_changes_view.rb
+++ b/lib/ladle/steward_changes_view.rb
@@ -11,18 +11,6 @@ def initialize(ref:, stewards_file:, file_filter: nil, changes: nil)
       @changes       = changes || []
     end
 
-    def empty?
-      @changes.empty?
-    end
-
-    def add_file_changes(file_changes)
-      file_changes.each do |file_change|
-        if @file_filter.include?(file_change.file.relative_path_from(@stewards_file.dirname))
-          @changes << file_change
-        end
-      end
-    end
-
     def ==(other)
       @ref == other.ref &&
         @stewards_file.to_s == other.stewards_file.to_s &&
diff --git a/lib/ladle/steward_rules.rb b/lib/ladle/steward_rules.rb
new file mode 100644
index 0000000..8618385
--- /dev/null
+++ b/lib/ladle/steward_rules.rb
@@ -0,0 +1,31 @@
+require 'ladle/file_filter'
+
+module Ladle
+  class StewardRules
+    attr_reader :ref, :stewards_file, :file_filter
+
+    def initialize(ref:, stewards_file:, file_filter: nil)
+      @ref           = ref
+      @file_filter   = file_filter || FileFilter.new
+      @stewards_file = Pathname.new(stewards_file)
+    end
+
+    def select_matching_file_changes(file_changes)
+      file_changes.select do |file_change|
+        @file_filter.include?(file_change.file.relative_path_from(@stewards_file.dirname))
+      end
+    end
+
+    def ==(other)
+      @ref == other.ref &&
+        @stewards_file == other.stewards_file &&
+        @file_filter == other.file_filter
+    end
+
+    def hash
+      [@ref, @stewards_file, @file_filter].hash
+    end
+
+    alias eql? ==
+  end
+end
diff --git a/test/factories/ladle/steward_rules.rb b/test/factories/ladle/steward_rules.rb
new file mode 100644
index 0000000..8b24b48
--- /dev/null
+++ b/test/factories/ladle/steward_rules.rb
@@ -0,0 +1,13 @@
+FactoryGirl.define do
+  factory :steward_rules, class: "Ladle::StewardRules" do
+    ref 'base'
+    stewards_file 'stewards.yml'
+    file_filter nil
+
+    initialize_with do
+      new(ref:           ref,
+          stewards_file: stewards_file,
+          file_filter:   file_filter)
+    end
+  end
+end
diff --git a/test/ladle/pull_handler_test.rb b/test/ladle/pull_handler_test.rb
index dae0848..363b9cd 100644
--- a/test/ladle/pull_handler_test.rb
+++ b/test/ladle/pull_handler_test.rb
@@ -427,14 +427,23 @@ class PullHandlerTest < ActiveSupport::TestCase
     Ladle::PullHandler.new(client, notifier).handle(@pull_request)
   end
 
-  test "resolve_stewards_scope" do
-    registry = {
-      'xanderstrike' => {
-        'stewards.yml'      => [build(:steward_changes_view, stewards_file: 'stewards.yml')],
-        'sub/stewards.yml'  => [build(:steward_changes_view, stewards_file: 'sub/stewards.yml')],
-        'sub3/stewards.yml' => [build(:steward_changes_view, stewards_file: 'sub3/stewards.yml')],
-      }
-    }
+  test "collect_changes" do
+    tree = Ladle::PullHandler::StewardTree.new('xanderstrike')
+
+    tree.add_rules(Ladle::StewardRules.new(ref:           'base',
+                                           stewards_file: 'stewards.yml',
+                                           file_filter:   Ladle::FileFilter.new))
+
+    tree.add_rules(Ladle::StewardRules.new(ref:           'base',
+                                           stewards_file: 'sub/stewards.yml',
+                                           file_filter:   Ladle::FileFilter.new))
+
+    tree.add_rules(Ladle::StewardRules.new(ref:           'base',
+                                           stewards_file: 'sub3/stewards.yml',
+                                           file_filter:   Ladle::FileFilter.new))
+
+    stewards_trees = {}
+    stewards_trees['xanderstrike'] = tree
 
     changed_files = Ladle::ChangedFiles.new
     changed_files.add_file_change(build(:file_change, file: 'stewards.yml'))
@@ -445,7 +454,7 @@ class PullHandlerTest < ActiveSupport::TestCase
     changed_files.add_file_change(build(:file_change, file: 'sub3/stewards.yml'))
 
     handler = Ladle::PullHandler.new(mock('client'), mock('notifier'))
-    resolved_stewards_registry = handler.send(:resolve_stewards_scope, registry, changed_files)
+    resolved_stewards_registry = handler.send(:collect_changes, stewards_trees, changed_files)
 
     expected_changes_view = build(:steward_changes_view,
       stewards_file: 'stewards.yml',
diff --git a/test/ladle/steward_changes_view_test.rb b/test/ladle/steward_changes_view_test.rb
index 82a0383..c4d9904 100644
--- a/test/ladle/steward_changes_view_test.rb
+++ b/test/ladle/steward_changes_view_test.rb
@@ -1,5 +1,4 @@
 require 'test_helper'
-require 'ladle/file_filter'
 
 class StewardChangesViewTest < ActiveSupport::TestCase
 
@@ -21,25 +20,4 @@ class StewardChangesViewTest < ActiveSupport::TestCase
     assert changes_view2.eql?(changes_view1)
     assert changes_view1.hash, changes_view2.hash
   end
-
-  test 'add_file_changes' do
-    view = build(:steward_changes_view,
-                 stewards_file: "base/stewards.yml",
-                 file_filter: Ladle::FileFilter.new(
-                   include_patterns: ["dir1/*.rb"],
-                   exclude_patterns: ["dir1/dir2/dir3/file.rb"]
-                 ))
-
-    file_changes = [
-      build(:file_change, file: "base/dir1/file.rb"),
-      build(:file_change, file: "base/dir1/dir2/dir3/file.rb"),
-      build(:file_change, file: "base/dir1/dir2/dir3/file.txt"),
-      build(:file_change, file: "base/dir1/dir2/dir3/file2.rb"),
-      build(:file_change, file: "base/dir2/teehee.rb"),
-    ]
-
-    view.add_file_changes(file_changes)
-
-    assert_equal [build(:file_change, file: "base/dir1/file.rb"), build(:file_change, file: "base/dir1/dir2/dir3/file2.rb")], view.changes
-  end
 end
diff --git a/test/ladle/steward_rules_test.rb b/test/ladle/steward_rules_test.rb
new file mode 100644
index 0000000..bf388c4
--- /dev/null
+++ b/test/ladle/steward_rules_test.rb
@@ -0,0 +1,15 @@
+require 'test_helper'
+
+class StewardRulesTest < ActiveSupport::TestCase
+
+  test 'value object' do
+    rules1 = build(:steward_rules)
+
+    rules2 = build(:steward_rules)
+
+    assert_equal rules1, rules2
+    assert rules1.eql?(rules2)
+    assert rules2.eql?(rules1)
+    assert rules1.hash, rules2.hash
+  end
+end

From 7c9146f09b6b79b35271533026d2efb8dfa969ba Mon Sep 17 00:00:00 2001
From: Donnie Tognazzini <donnie.tognazzini@appfolio.com>
Date: Tue, 15 Dec 2015 13:05:11 -0800
Subject: [PATCH 13/15] De-nesting StewardTree and ChangesViews and making
 ChangesView enumerable

---
 lib/ladle/changes_view.rb       | 44 +++++++++++++++++
 lib/ladle/pull_handler.rb       | 77 ++++--------------------------
 lib/ladle/steward_tree.rb       | 31 ++++++++++++
 test/ladle/changes_view_test.rb | 83 +++++++++++++++++++++++++++++++++
 test/ladle/pull_handler_test.rb |  4 +-
 test/ladle/steward_tree_test.rb | 41 ++++++++++++++++
 6 files changed, 212 insertions(+), 68 deletions(-)
 create mode 100644 lib/ladle/changes_view.rb
 create mode 100644 lib/ladle/steward_tree.rb
 create mode 100644 test/ladle/changes_view_test.rb
 create mode 100644 test/ladle/steward_tree_test.rb

diff --git a/lib/ladle/changes_view.rb b/lib/ladle/changes_view.rb
new file mode 100644
index 0000000..0610104
--- /dev/null
+++ b/lib/ladle/changes_view.rb
@@ -0,0 +1,44 @@
+module Ladle
+  class ChangesView
+    include Enumerable
+
+    RulesChanges = Struct.new(:rules, :changes)
+
+    def initialize
+      @paths = {}
+    end
+
+    def empty?
+      @paths.values.all?(&:empty?)
+    end
+
+    def each
+      sorted_paths = @paths.keys.sort
+      sorted_paths.each do |path_key|
+        rules_and_changes_list = @paths[path_key]
+
+        rules_and_changes_list.each do |rules_and_changes|
+          yield rules_and_changes.rules, rules_and_changes.changes
+        end
+      end
+    end
+
+    def add_changes(rules, changes)
+      @paths[rules.stewards_file.to_s] ||= []
+
+      add_changes_to(@paths[rules.stewards_file.to_s], rules, changes)
+    end
+
+    private
+
+    def add_changes_to(rules_and_changes_collection, rules, changes)
+      changes_already_exist = rules_and_changes_collection.any? do |rules_changes|
+        rules_changes.changes == changes
+      end
+
+      unless changes_already_exist
+        rules_and_changes_collection << RulesChanges.new(rules, changes)
+      end
+    end
+  end
+end
diff --git a/lib/ladle/pull_handler.rb b/lib/ladle/pull_handler.rb
index 8d2b0b1..2ae0caf 100644
--- a/lib/ladle/pull_handler.rb
+++ b/lib/ladle/pull_handler.rb
@@ -1,5 +1,7 @@
 require 'ladle/stewards_file_parser'
 require 'ladle/steward_rules'
+require 'ladle/steward_tree'
+require 'ladle/steward_changes_view'
 
 module Ladle
   class PullHandler
@@ -41,66 +43,6 @@ def collect_stewards_rules(pull_request, pr_info, pr_files)
       rules
     end
 
-    class ChangesView
-      attr_reader :paths
-
-      RulesChanges = Struct.new(:rules, :changes)
-
-      def initialize
-        @paths = {}
-      end
-
-      def empty?
-        @paths.values.all?(&:empty?)
-      end
-
-      def add_changes(rules, changes)
-        @paths[rules.stewards_file.to_s] ||= []
-
-        add_changes_to(@paths[rules.stewards_file.to_s], rules, changes)
-      end
-
-      private
-
-      def add_changes_to(rules_and_changes_collection, rules, changes)
-        changes_already_exist = rules_and_changes_collection.any? do |rules_changes|
-          rules_changes.changes == changes
-        end
-
-        unless changes_already_exist
-          rules_and_changes_collection << RulesChanges.new(rules, changes)
-        end
-      end
-    end
-
-    class StewardTree
-      attr_reader :github_username
-
-      def initialize(github_username)
-        @github_username = github_username
-        @rules           = []
-      end
-
-      def add_rules(rules)
-        @rules << rules
-      end
-
-      def changes(pull_request_files)
-        changes_view = ChangesView.new
-
-        @rules.each do |rules|
-          file_changes = pull_request_files.file_changes_in(rules.stewards_file.dirname)
-          file_changes = rules.select_matching_file_changes(file_changes)
-
-          unless file_changes.empty?
-            changes_view.add_changes(rules, file_changes)
-          end
-        end
-
-        changes_view
-      end
-    end
-
     def register_stewards(stewards_rules_map, stewards_file_path, sha)
       contents = @client.contents(path: stewards_file_path.to_s, ref: sha)
       stewards_file = StewardsFileParser.parse(contents)
@@ -135,13 +77,14 @@ def collect_changes(stewards_trees, pull_request_files)
 
     def map_to_old_stewards_tree_change_view(changes_view)
       old_datastructure = {}
-      changes_view.paths.each do |stewards_file_path, rules_with_changes_list|
-        old_datastructure[stewards_file_path] = rules_with_changes_list.map do |rule_with_changes|
-          Ladle::StewardChangesView.new(ref:           rule_with_changes.rules.ref,
-                                        stewards_file: rule_with_changes.rules.stewards_file,
-                                        file_filter:   rule_with_changes.rules.file_filter,
-                                        changes:       rule_with_changes.changes)
-        end
+      changes_view.each do |rules, changes|
+        old_change_view = Ladle::StewardChangesView.new(ref:           rules.ref,
+                                                        stewards_file: rules.stewards_file,
+                                                        file_filter:   rules.file_filter,
+                                                        changes:       changes)
+
+        old_datastructure[rules.stewards_file.to_s] ||= []
+        old_datastructure[rules.stewards_file.to_s] << old_change_view
       end
 
       old_datastructure
diff --git a/lib/ladle/steward_tree.rb b/lib/ladle/steward_tree.rb
new file mode 100644
index 0000000..d3b2d5e
--- /dev/null
+++ b/lib/ladle/steward_tree.rb
@@ -0,0 +1,31 @@
+require 'ladle/changes_view'
+
+module Ladle
+  class StewardTree
+    attr_reader :github_username
+
+    def initialize(github_username)
+      @github_username = github_username
+      @rules           = []
+    end
+
+    def add_rules(rules)
+      @rules << rules
+    end
+
+    def changes(pull_request_files)
+      changes_view = ChangesView.new
+
+      @rules.each do |rules|
+        file_changes = pull_request_files.file_changes_in(rules.stewards_file.dirname)
+        file_changes = rules.select_matching_file_changes(file_changes)
+
+        unless file_changes.empty?
+          changes_view.add_changes(rules, file_changes)
+        end
+      end
+
+      changes_view
+    end
+  end
+end
diff --git a/test/ladle/changes_view_test.rb b/test/ladle/changes_view_test.rb
new file mode 100644
index 0000000..d304f84
--- /dev/null
+++ b/test/ladle/changes_view_test.rb
@@ -0,0 +1,83 @@
+require 'test_helper'
+require 'ladle/steward_rules'
+require 'ladle/changes_view'
+require 'ladle/changed_files'
+
+class ChangesViewTest < ActiveSupport::TestCase
+
+  setup :create_file_changes
+
+  test 'empty?' do
+    changes_view = Ladle::ChangesView.new
+    assert_predicate changes_view, :empty?
+
+    rules = Ladle::StewardRules.new(ref:           'base',
+                                    stewards_file: 'stewards.yml')
+
+    changes_view.add_changes(rules, @file_changes)
+    refute_predicate changes_view, :empty?
+  end
+
+  test 'enumerates in order' do
+    changes_view = Ladle::ChangesView.new
+
+    rules_list = [
+      Ladle::StewardRules.new(ref:           'base',
+                              stewards_file: 'sub1/sub2/stewards.yml'),
+      Ladle::StewardRules.new(ref:           'base',
+                              stewards_file: 'stewards.yml'),
+      Ladle::StewardRules.new(ref:           'base',
+                              stewards_file: 'sub3/stewards.yml'),
+    ]
+
+    rules_list.each do |rules|
+      changes_view.add_changes(rules, @file_changes)
+    end
+
+    expected_order = [
+      'stewards.yml',
+      'sub1/sub2/stewards.yml',
+      'sub3/stewards.yml',
+    ]
+
+    assert_equal expected_order, changes_view.to_a.map(&:first).map(&:stewards_file).map(&:to_s)
+  end
+
+  test 'rules with same file and same changes are collapsed' do
+    changes_view = Ladle::ChangesView.new
+
+    base_rules = Ladle::StewardRules.new(ref:           'base',
+                                         stewards_file: 'stewards.yml')
+
+    changes_view.add_changes(base_rules, @file_changes)
+
+    bleh_rules = Ladle::StewardRules.new(ref:           'bleh',
+                                         stewards_file: 'stewards.yml')
+
+    changes_view.add_changes(bleh_rules, @file_changes)
+    assert_equal [[base_rules, @file_changes]], changes_view.to_a
+  end
+
+  test 'rules with same changes but different file are not collapsed' do
+    changes_view = Ladle::ChangesView.new
+
+    base_rules = Ladle::StewardRules.new(ref:           'base',
+                                         stewards_file: 'stewards.yml')
+
+    changes_view.add_changes(base_rules, @file_changes)
+
+    bleh_rules = Ladle::StewardRules.new(ref:           'bleh',
+                                         stewards_file: 'sub/stewards.yml')
+
+    changes_view.add_changes(bleh_rules, @file_changes)
+    assert_equal [[base_rules, @file_changes], [bleh_rules, @file_changes]], changes_view.to_a
+  end
+
+  private
+
+  def create_file_changes
+    @file_changes = []
+    @file_changes << build(:file_change, file: 'stewards.yml')
+    @file_changes << build(:file_change, file: 'one.rb')
+  end
+end
diff --git a/test/ladle/pull_handler_test.rb b/test/ladle/pull_handler_test.rb
index 363b9cd..d2ba0b7 100644
--- a/test/ladle/pull_handler_test.rb
+++ b/test/ladle/pull_handler_test.rb
@@ -4,6 +4,8 @@
 require 'ladle/pull_request_info'
 require 'ladle/changed_files'
 require 'ladle/file_filter'
+require 'ladle/steward_tree'
+require 'ladle/steward_rules'
 
 require 'ladle/stubbed_repo_client'
 
@@ -428,7 +430,7 @@ class PullHandlerTest < ActiveSupport::TestCase
   end
 
   test "collect_changes" do
-    tree = Ladle::PullHandler::StewardTree.new('xanderstrike')
+    tree = Ladle::StewardTree.new('xanderstrike')
 
     tree.add_rules(Ladle::StewardRules.new(ref:           'base',
                                            stewards_file: 'stewards.yml',
diff --git a/test/ladle/steward_tree_test.rb b/test/ladle/steward_tree_test.rb
new file mode 100644
index 0000000..f85f0fd
--- /dev/null
+++ b/test/ladle/steward_tree_test.rb
@@ -0,0 +1,41 @@
+require 'test_helper'
+
+require 'ladle/steward_tree'
+require 'ladle/steward_rules'
+require 'ladle/changed_files'
+
+class StewardTreeTest < ActiveSupport::TestCase
+  test 'changes selected from rules' do
+    changed_files = Ladle::ChangedFiles.new
+    changed_files.add_file_change(build(:file_change, file: 'stewards.yml'))
+    changed_files.add_file_change(build(:file_change, file: 'one.rb'))
+    changed_files.add_file_change(build(:file_change, file: 'sub/marine.rb'))
+    changed_files.add_file_change(build(:file_change, file: 'sub/stewards.yml'))
+    changed_files.add_file_change(build(:file_change, file: 'sub2/sandwich'))
+    changed_files.add_file_change(build(:file_change, file: 'sub3/stewards.yml'))
+
+    tree = Ladle::StewardTree.new('xanderstrike')
+
+    sub_rules = Ladle::StewardRules.new(ref:           'base',
+                                        stewards_file: 'sub/stewards.yml')
+
+    tree.add_rules(sub_rules)
+
+    sub4_rules = Ladle::StewardRules.new(ref:           'base',
+                                        stewards_file: 'sub4/stewards.yml')
+
+    tree.add_rules(sub4_rules)
+
+    expected_rules_and_changes = [
+      [
+        sub_rules,
+        [
+          build(:file_change, file: 'sub/marine.rb'),
+          build(:file_change, file: 'sub/stewards.yml')
+        ]
+      ]
+    ]
+
+    assert_equal expected_rules_and_changes, tree.changes(changed_files).to_a
+  end
+end

From fff8f6d779c26dc8b1ef553f70678895ba3e4707 Mon Sep 17 00:00:00 2001
From: Donnie Tognazzini <donnie.tognazzini@appfolio.com>
Date: Tue, 15 Dec 2015 13:12:59 -0800
Subject: [PATCH 14/15] Adding test case for unmatched rules

---
 test/ladle/pull_handler_test.rb | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/test/ladle/pull_handler_test.rb b/test/ladle/pull_handler_test.rb
index d2ba0b7..87b022f 100644
--- a/test/ladle/pull_handler_test.rb
+++ b/test/ladle/pull_handler_test.rb
@@ -444,6 +444,10 @@ class PullHandlerTest < ActiveSupport::TestCase
                                            stewards_file: 'sub3/stewards.yml',
                                            file_filter:   Ladle::FileFilter.new))
 
+    tree.add_rules(Ladle::StewardRules.new(ref:           'base',
+                                           stewards_file: 'sub4/stewards.yml',
+                                           file_filter:   Ladle::FileFilter.new))
+
     stewards_trees = {}
     stewards_trees['xanderstrike'] = tree
 

From b753974a0a734e76ab203502ed155a6093267776 Mon Sep 17 00:00:00 2001
From: Donnie Tognazzini <donnie.tognazzini@appfolio.com>
Date: Tue, 15 Dec 2015 18:07:28 -0800
Subject: [PATCH 15/15] Dropping StewardChangesView and plumbing ChangesView
 datastructure through

---
 app/mailers/user_mailer.rb                   |   6 +-
 app/views/user_mailer/notify.html.erb        |   8 +-
 app/views/user_mailer/notify.text.erb        |   8 +-
 lib/ladle.rb                                 |   2 +-
 lib/ladle/changes_view.rb                    |  10 +
 lib/ladle/pull_handler.rb                    |  24 +-
 lib/ladle/steward_changes_view.rb            |  27 --
 lib/ladle/steward_notifier.rb                |   8 +-
 lib/ladle/steward_tree.rb                    |   6 +-
 lib/ladle/test_data.rb                       |  48 +-
 test/factories/ladle/changes_view.rb         |  13 +
 test/factories/ladle/steward_changes_view.rb |  15 -
 test/ladle/changes_view_test.rb              |  33 ++
 test/ladle/pull_handler_test.rb              | 453 ++++++++++---------
 test/ladle/steward_changes_view_test.rb      |  23 -
 test/ladle/steward_notifier_test.rb          |   6 +-
 test/ladle/steward_tree_test.rb              |   2 +-
 test/mailers/previews/user_mailer_preview.rb |   4 +-
 test/mailers/user_mailer_test.rb             |   4 +-
 19 files changed, 361 insertions(+), 339 deletions(-)
 delete mode 100644 lib/ladle/steward_changes_view.rb
 create mode 100644 test/factories/ladle/changes_view.rb
 delete mode 100644 test/factories/ladle/steward_changes_view.rb
 delete mode 100644 test/ladle/steward_changes_view_test.rb

diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb
index 3bc47e8..d40a511 100644
--- a/app/mailers/user_mailer.rb
+++ b/app/mailers/user_mailer.rb
@@ -9,10 +9,10 @@ def title
     end
   end
 
-  def notify(user:, repository:, pull_request:, stewards_file_map:)
+  def notify(user:, repository:, pull_request:, changes_view:)
     @pull_request = PullRequestPresenter.new(pull_request)
-    @stewards_file_map = stewards_file_map
-    @user = user
+    @changes_view = changes_view
+    @user         = user
 
     mail(to:      user.email,
          subject: "[#{repository}] Ladle Alert: #{@pull_request.title}")
diff --git a/app/views/user_mailer/notify.html.erb b/app/views/user_mailer/notify.html.erb
index bb4b2a8..584e72b 100644
--- a/app/views/user_mailer/notify.html.erb
+++ b/app/views/user_mailer/notify.html.erb
@@ -9,17 +9,15 @@
 <p>You were notified of this pull request because of the following changes:</p>
 
 <ul>
-  <% @stewards_file_map.each do |stewards_file_path, steward_change_views| %>
-    <% steward_change_views.each do |steward_change_view| %>
+  <% @changes_view.each do |rules, changes| %>
   <li>
-    <p>Changes monitored by <%= steward_change_view.ref %>/<%= steward_change_view.stewards_file %>:</p>
+    <p>Changes monitored by <%= rules.ref %>/<%= rules.stewards_file %>:</p>
     <ul>
-      <% steward_change_view.changes.each do |change| %>
+      <% changes.each do |change| %>
         <li><strong><%= change.status_initial %></strong> <%= change.file %> (<%= change.changes_count %>)</li>
       <% end %>
     </ul>
   </li>
-    <% end %>
   <% end %>
 </ul>
 
diff --git a/app/views/user_mailer/notify.text.erb b/app/views/user_mailer/notify.text.erb
index dd70846..1a14a5c 100644
--- a/app/views/user_mailer/notify.text.erb
+++ b/app/views/user_mailer/notify.text.erb
@@ -10,15 +10,13 @@ You can view, comment on, or merge this pull request online at:
 
 You were notified of this pull request because of the following changes:
 
-<% @stewards_file_map.each do |stewards_file_path, steward_change_views| %>
-  <% steward_change_views.each do |steward_change_view| %>
-  Changes monitored by <%= steward_change_view.ref %>/<%= steward_change_view.stewards_file %>:
+<% @changes_view.each do |rules, changes| %>
+  Changes monitored by <%= rules.ref %>/<%= rules.stewards_file %>:
 
-  <% steward_change_view.changes.each do |change| %>
+  <% changes.each do |change| %>
     * <%= change.status_initial %> <%= change.file %> (<%= change.changes_count %>)
   <% end %>
 
-  <% end %>
 <% end %>
 ---------------------------------------------
 You received this email from Ladle for account <%= @user.github_username %>. Update your notification settings at <%= edit_user_settings_url %>.
diff --git a/lib/ladle.rb b/lib/ladle.rb
index cd7aedf..a09134b 100644
--- a/lib/ladle.rb
+++ b/lib/ladle.rb
@@ -2,7 +2,7 @@ module Ladle
   extend ActiveSupport::Autoload
 
   autoload :FileChange
-  autoload :StewardChangesView
+  autoload :ChangesView
   autoload :StewardRules
 end
 
diff --git a/lib/ladle/changes_view.rb b/lib/ladle/changes_view.rb
index 0610104..77aa555 100644
--- a/lib/ladle/changes_view.rb
+++ b/lib/ladle/changes_view.rb
@@ -12,6 +12,12 @@ def empty?
       @paths.values.all?(&:empty?)
     end
 
+    def ==(other)
+      @paths == other.paths
+    end
+
+    alias eql? ==
+
     def each
       sorted_paths = @paths.keys.sort
       sorted_paths.each do |path_key|
@@ -29,6 +35,10 @@ def add_changes(rules, changes)
       add_changes_to(@paths[rules.stewards_file.to_s], rules, changes)
     end
 
+    protected
+
+    attr_reader :paths
+
     private
 
     def add_changes_to(rules_and_changes_collection, rules, changes)
diff --git a/lib/ladle/pull_handler.rb b/lib/ladle/pull_handler.rb
index 2ae0caf..f591009 100644
--- a/lib/ladle/pull_handler.rb
+++ b/lib/ladle/pull_handler.rb
@@ -1,7 +1,6 @@
 require 'ladle/stewards_file_parser'
 require 'ladle/steward_rules'
 require 'ladle/steward_tree'
-require 'ladle/steward_changes_view'
 
 module Ladle
   class PullHandler
@@ -15,7 +14,7 @@ def handle(pull_request)
 
       pr_files = @client.pull_request_files(pull_request.number)
 
-      stewards_trees = collect_stewards_rules(pull_request, pr_info, pr_files)
+      stewards_trees = collect_stewards_rules(pr_info, pr_files)
 
       stewards_registry = collect_changes(stewards_trees, pr_files)
 
@@ -29,7 +28,7 @@ def handle(pull_request)
 
     private
 
-    def collect_stewards_rules(pull_request, pr_info, pr_files)
+    def collect_stewards_rules(pr_info, pr_files)
       rules = {}
 
       pr_files.directories.each do |directory|
@@ -52,7 +51,7 @@ def register_stewards(stewards_rules_map, stewards_file_path, sha)
                                         stewards_file: stewards_file_path,
                                         file_filter:   steward_config.file_filter)
 
-        stewards_rules_map[steward_config.github_username] ||= StewardTree.new(steward_config.github_username)
+        stewards_rules_map[steward_config.github_username] ||= StewardTree.new
         stewards_rules_map[steward_config.github_username].add_rules(rules)
       end
     rescue Ladle::RemoteFileNotFound
@@ -68,26 +67,11 @@ def collect_changes(stewards_trees, pull_request_files)
         changes_view = steward_tree.changes(pull_request_files)
 
         unless changes_view.empty?
-          output[github_username] = map_to_old_stewards_tree_change_view(changes_view)
+          output[github_username] = changes_view
         end
       end
 
       output
     end
-
-    def map_to_old_stewards_tree_change_view(changes_view)
-      old_datastructure = {}
-      changes_view.each do |rules, changes|
-        old_change_view = Ladle::StewardChangesView.new(ref:           rules.ref,
-                                                        stewards_file: rules.stewards_file,
-                                                        file_filter:   rules.file_filter,
-                                                        changes:       changes)
-
-        old_datastructure[rules.stewards_file.to_s] ||= []
-        old_datastructure[rules.stewards_file.to_s] << old_change_view
-      end
-
-      old_datastructure
-    end
   end
 end
diff --git a/lib/ladle/steward_changes_view.rb b/lib/ladle/steward_changes_view.rb
deleted file mode 100644
index fe9f2cc..0000000
--- a/lib/ladle/steward_changes_view.rb
+++ /dev/null
@@ -1,27 +0,0 @@
-require 'ladle/file_filter'
-
-module Ladle
-  class StewardChangesView
-    attr_reader :ref, :stewards_file, :file_filter, :changes
-
-    def initialize(ref:, stewards_file:, file_filter: nil, changes: nil)
-      @ref           = ref
-      @file_filter   = file_filter || FileFilter.new
-      @stewards_file = Pathname.new(stewards_file)
-      @changes       = changes || []
-    end
-
-    def ==(other)
-      @ref == other.ref &&
-        @stewards_file.to_s == other.stewards_file.to_s &&
-        @file_filter == other.file_filter &&
-        @changes.to_set == other.changes.to_set
-    end
-
-    def hash
-      [@ref, @stewards_file, @file_filter, @changes].hash
-    end
-
-    alias eql? ==
-  end
-end
diff --git a/lib/ladle/steward_notifier.rb b/lib/ladle/steward_notifier.rb
index a0e4a4e..fed5f16 100644
--- a/lib/ladle/steward_notifier.rb
+++ b/lib/ladle/steward_notifier.rb
@@ -7,11 +7,11 @@ def initialize(repository_name, pull_request)
 
     def notify(stewards_changes)
       notified_users = []
-      stewards_changes.each do |github_username, steward_changes_views|
+      stewards_changes.each do |github_username, changes_view|
         user = User.find_by_github_username(github_username)
 
         if user && ! user_has_been_notified?(user)
-          send_email(user, steward_changes_views)
+          send_email(user, changes_view)
           notified_users << user
         end
       end
@@ -25,8 +25,8 @@ def user_has_been_notified?(user)
       user.notifications.where(pull_request: @pull_request).exists?
     end
 
-    def send_email(user, stewards_file_map)
-      UserMailer.notify(user: user, repository: @repository_name, pull_request: @pull_request, stewards_file_map: stewards_file_map).deliver_now
+    def send_email(user, changes_view)
+      UserMailer.notify(user: user, repository: @repository_name, pull_request: @pull_request, changes_view: changes_view).deliver_now
     end
 
     def create_notification(users)
diff --git a/lib/ladle/steward_tree.rb b/lib/ladle/steward_tree.rb
index d3b2d5e..ebf8ff2 100644
--- a/lib/ladle/steward_tree.rb
+++ b/lib/ladle/steward_tree.rb
@@ -2,11 +2,9 @@
 
 module Ladle
   class StewardTree
-    attr_reader :github_username
 
-    def initialize(github_username)
-      @github_username = github_username
-      @rules           = []
+    def initialize
+      @rules = []
     end
 
     def add_rules(rules)
diff --git a/lib/ladle/test_data.rb b/lib/ladle/test_data.rb
index 00f83f1..357cc64 100644
--- a/lib/ladle/test_data.rb
+++ b/lib/ladle/test_data.rb
@@ -2,27 +2,33 @@ module Ladle
   module TestData
     extend FactoryGirl::Syntax::Methods
 
-    def self.create_steward_changes_views
-      {
-        'app/stewards.yml'        => [build(:steward_changes_view,
-                                            stewards_file: 'app/stewards.yml',
-                                            changes:       [
-                                                             build(:file_change, status: :removed, file: "app/removed_file.rb", deletions: 6),
-                                                             build(:file_change, status: :modified, file: "app/modified_file.rb", deletions: 3, additions: 3),
-                                                             build(:file_change, status: :added, file: "app/new_file.rb", additions: 6),
-                                                           ]),
-                                      build(:steward_changes_view,
-                                            ref:           'branch',
-                                            stewards_file: 'app/stewards.yml',
-                                            changes:       [
-                                                             build(:file_change, status: :modified, file: "app/blah.rb", deletions: 3, additions: 3),
-                                                           ])],
-        'lib/closet/stewards.yml' => [build(:steward_changes_view,
-                                            stewards_file: 'lib/closet/stewards.yml',
-                                            changes:       [
-                                                             build(:file_change, status: :added, file: "lib/closet/top_shelf/new_file.rb", additions: 6),
-                                                           ])],
-      }
+    def self.create_changes_view
+      build(:changes_view,
+            changes: [
+                       {
+                         rules:   Ladle::StewardRules.new(ref:           'base',
+                                                          stewards_file: 'app/stewards.yml'),
+                         changes: [
+                                    build(:file_change, status: :removed, file: "app/removed_file.rb", deletions: 6),
+                                    build(:file_change, status: :modified, file: "app/modified_file.rb", deletions: 3, additions: 3),
+                                    build(:file_change, status: :added, file: "app/new_file.rb", additions: 6),
+                                  ]
+                       },
+                       {
+                         rules:   Ladle::StewardRules.new(ref:           'branch',
+                                                          stewards_file: 'app/stewards.yml'),
+                         changes: [
+                                    build(:file_change, status: :modified, file: "app/blah.rb", deletions: 3, additions: 3),
+                                  ]
+                       },
+                       {
+                         rules:   Ladle::StewardRules.new(ref:           'base',
+                                                          stewards_file: 'lib/closet/stewards.yml'),
+                         changes: [
+                                    build(:file_change, status: :added, file: "lib/closet/top_shelf/new_file.rb", additions: 6),
+                                  ]
+                       },
+                     ])
     end
   end
 end
diff --git a/test/factories/ladle/changes_view.rb b/test/factories/ladle/changes_view.rb
new file mode 100644
index 0000000..0796793
--- /dev/null
+++ b/test/factories/ladle/changes_view.rb
@@ -0,0 +1,13 @@
+FactoryGirl.define do
+  factory :changes_view, class: "Ladle::ChangesView" do
+
+    initialize_with do
+      view = new
+      attributes[:changes].each do |rules_and_changes|
+        view.add_changes(rules_and_changes[:rules], rules_and_changes[:changes])
+      end
+
+      view
+    end
+  end
+end
diff --git a/test/factories/ladle/steward_changes_view.rb b/test/factories/ladle/steward_changes_view.rb
deleted file mode 100644
index ad29ebd..0000000
--- a/test/factories/ladle/steward_changes_view.rb
+++ /dev/null
@@ -1,15 +0,0 @@
-FactoryGirl.define do
-  factory :steward_changes_view, class: "Ladle::StewardChangesView" do
-    ref 'base'
-    stewards_file 'stewards.yml'
-    file_filter nil
-    changes nil
-
-    initialize_with do
-      new(ref:           ref,
-          stewards_file: stewards_file,
-          file_filter:   file_filter,
-          changes:       changes)
-    end
-  end
-end
diff --git a/test/ladle/changes_view_test.rb b/test/ladle/changes_view_test.rb
index d304f84..0d3bf3c 100644
--- a/test/ladle/changes_view_test.rb
+++ b/test/ladle/changes_view_test.rb
@@ -7,6 +7,39 @@ class ChangesViewTest < ActiveSupport::TestCase
 
   setup :create_file_changes
 
+  test 'comparable' do
+    changes_view1 = build(:changes_view,
+                          changes: [
+                                     {
+                                       rules:   Ladle::StewardRules.new(ref:           'base',
+                                                                        stewards_file: 'app/stewards.yml'),
+                                       changes: [
+                                                  build(:file_change, status: :modified, file: "app/modified_file.rb"),
+                                                  build(:file_change, status: :added, file: "app/new_file.rb"),
+                                                  build(:file_change, status: :removed, file: "app/removed_file.rb"),
+                                                ]
+                                     }
+                                   ])
+
+
+    changes_view2 = build(:changes_view,
+                          changes: [
+                                     {
+                                       rules:   Ladle::StewardRules.new(ref:           'base',
+                                                                        stewards_file: 'app/stewards.yml'),
+                                       changes: [
+                                                  build(:file_change, status: :modified, file: "app/modified_file.rb"),
+                                                  build(:file_change, status: :added, file: "app/new_file.rb"),
+                                                  build(:file_change, status: :removed, file: "app/removed_file.rb"),
+                                                ]
+                                     }
+                                   ])
+
+    assert_equal changes_view1, changes_view2
+    assert changes_view1.eql?(changes_view2)
+    assert changes_view2.eql?(changes_view1)
+  end
+
   test 'empty?' do
     changes_view = Ladle::ChangesView.new
     assert_predicate changes_view, :empty?
diff --git a/test/ladle/pull_handler_test.rb b/test/ladle/pull_handler_test.rb
index 87b022f..f988969 100644
--- a/test/ladle/pull_handler_test.rb
+++ b/test/ladle/pull_handler_test.rb
@@ -56,45 +56,55 @@ class PullHandlerTest < ActiveSupport::TestCase
       YAML
     end
 
-    expected_stewards_changes_view = build(:steward_changes_view,
-                                           ref: 'base_head',
-                                           stewards_file: 'stewards.yml',
-                                           changes:       [
-                                                            build(:file_change, status: :added, file: 'one.rb', additions: 1),
-                                                            build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1, deletions: 1),
-                                                          ])
-
-    expected_sub_stewards_changes_view = build(:steward_changes_view,
-                                               ref: 'base_head',
-                                               stewards_file: 'sub/stewards.yml',
-                                               changes:       [
-                                                                build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1, deletions: 1),
-                                                              ])
     notifier = mock
     notifier.expects(:notify)
-      .with(deep_hash({
-              'xanderstrike'      => {
-                'sub/stewards.yml' => [expected_sub_stewards_changes_view],
-                'stewards.yml'     => [expected_stewards_changes_view]
-              },
-              'fadsfadsfadsfadsf' => {
-                'sub/stewards.yml' => [expected_sub_stewards_changes_view]
-              },
-              'bob'               => {
-                'stewards.yml' => [build(:steward_changes_view,
-                                         ref: 'base_head',
-                                         stewards_file: 'stewards.yml',
-                                         file_filter:   Ladle::FileFilter.new(
-                                           include_patterns: ["**.rb"],
-                                           exclude_patterns: ["**.txt"]
-                                         ),
-                                         changes:       [
-                                                          build(:file_change, status: :added, file: 'one.rb', additions: 1),
-                                                          build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1, deletions: 1),
-                                                        ])
-                ]
-              }
-            }))
+      .with(deep_hash(
+              {
+                'xanderstrike'      => build(:changes_view,
+                                             changes: [
+                                                        {
+                                                          rules:   Ladle::StewardRules.new(ref:           'base_head',
+                                                                                           stewards_file: 'stewards.yml'),
+                                                          changes: [
+                                                                     build(:file_change, status: :added, file: 'one.rb', additions: 1),
+                                                                     build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1, deletions: 1),
+                                                                   ]
+                                                        },
+                                                        {
+                                                          rules:   Ladle::StewardRules.new(ref:           'base_head',
+                                                                                           stewards_file: 'sub/stewards.yml'),
+                                                          changes: [
+                                                                     build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1, deletions: 1),
+                                                                   ]
+                                                        },
+                                                      ]),
+                'fadsfadsfadsfadsf' => build(:changes_view,
+                                             changes: [
+                                                        {
+                                                          rules:   Ladle::StewardRules.new(ref:           'base_head',
+                                                                                           stewards_file: 'sub/stewards.yml'),
+                                                          changes: [
+                                                                     build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1, deletions: 1),
+                                                                   ]
+                                                        },
+                                                      ]),
+                'bob'               => build(:changes_view,
+                                             changes: [
+                                                        {
+                                                          rules:   Ladle::StewardRules.new(ref:           'base_head',
+                                                                                           stewards_file: 'stewards.yml',
+                                                                                           file_filter:   Ladle::FileFilter.new(
+                                                                                             include_patterns: ["**.rb"],
+                                                                                             exclude_patterns: ["**.txt"]
+                                                                                           )),
+                                                          changes: [
+                                                                     build(:file_change, status: :added, file: 'one.rb', additions: 1),
+                                                                     build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1, deletions: 1),
+                                                                   ]
+                                                        }
+                                                      ])
+
+              }))
 
     Ladle::PullHandler.new(client, notifier).handle(@pull_request)
   end
@@ -119,35 +129,35 @@ class PullHandlerTest < ActiveSupport::TestCase
           include: file2.txt
     YAML
 
-    base_changes = build(:steward_changes_view,
-                         ref: 'base_head',
-                         stewards_file: 'stewards.yml',
-                         file_filter:   Ladle::FileFilter.new(
-                           include_patterns: ["file1.txt"]
-                         ),
-                         changes:       [
-                                          build(:file_change, status: :added, file: 'file1.txt', additions: 1)
-                                        ])
-
-    branch_changes = build(:steward_changes_view,
-                           ref: 'branch_head',
-                           stewards_file: 'stewards.yml',
-                           file_filter:   Ladle::FileFilter.new(
-                             include_patterns: ["file2.txt"]
-                           ),
-                           changes:       [
-                                            build(:file_change, status: :added, file: 'file2.txt', additions: 1)
-                                          ])
-
     notifier = mock
     notifier.expects(:notify)
       .with(deep_hash({
-                        'someguy' => {
-                          'stewards.yml' => [
-                            base_changes,
-                            branch_changes,
-                          ],
-                        }}))
+                        'someguy' => build(:changes_view,
+                                           changes: [
+                                                      {
+                                                        rules:   Ladle::StewardRules.new(ref:           'base_head',
+                                                                                         stewards_file: 'stewards.yml',
+                                                                                         file_filter:   Ladle::FileFilter.new(
+                                                                                           include_patterns: ["file1.txt"]
+                                                                                         ),
+                                                        ),
+                                                        changes: [
+                                                                   build(:file_change, status: :added, file: 'file1.txt', additions: 1)
+                                                                 ]
+                                                      },
+                                                      {
+                                                        rules:   Ladle::StewardRules.new(ref:           'branch_head',
+                                                                                         stewards_file: 'stewards.yml',
+                                                                                         file_filter:   Ladle::FileFilter.new(
+                                                                                           include_patterns: ["file2.txt"]
+                                                                                         ),
+                                                        ),
+                                                        changes: [
+                                                                   build(:file_change, status: :added, file: 'file2.txt', additions: 1)
+                                                                 ]
+                                                      },
+                                                    ])
+                      }))
 
     Ladle::PullHandler.new(client, notifier).handle(@pull_request)
   end
@@ -172,22 +182,26 @@ class PullHandlerTest < ActiveSupport::TestCase
           include: file1.*
     YAML
 
-    changes = build(:steward_changes_view,
-                    ref: 'base_head',
-                    stewards_file: 'stewards.yml',
-                    file_filter:   Ladle::FileFilter.new(
-                      include_patterns: ["file1.txt"]
-                    ),
-                    changes:       [
-                                     build(:file_change, status: :added, file: 'file1.txt', additions: 1)
-                                   ])
+    changes = build(:changes_view,
+                    changes: [
+                               {
+                                 rules:   Ladle::StewardRules.new(ref:           'base_head',
+                                                                  stewards_file: 'stewards.yml',
+                                                                  file_filter:   Ladle::FileFilter.new(
+                                                                    include_patterns: ["file1.txt"]
+                                                                  ),
+                                 ),
+                                 changes: [
+                                            build(:file_change, status: :added, file: 'file1.txt', additions: 1)
+                                          ]
+                               }
+                             ])
 
     notifier = mock
     notifier.expects(:notify)
       .with(deep_hash({
-                        'someguy' => {
-                          'stewards.yml' => [changes],
-                        }}))
+                        'someguy' => changes
+                      }))
 
     Ladle::PullHandler.new(client, notifier).handle(@pull_request)
   end
@@ -236,70 +250,88 @@ class PullHandlerTest < ActiveSupport::TestCase
       YAML
     end
 
-    expected_stewards_changes_view = build(:steward_changes_view,
-                                           ref: 'base_head',
-                                           stewards_file: 'stewards.yml',
-                                           changes:       [
-                                                            build(:file_change, status: :removed, file: 'stewards.yml', deletions: 1),
-                                                            build(:file_change, status: :added, file: 'one.rb', additions: 1),
-                                                            build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1),
-                                                            build(:file_change, status: :modified, file: 'sub/stewards.yml', additions: 1),
-                                                            build(:file_change, status: :removed, file: 'sub2/sandwich', deletions: 1),
-                                                            build(:file_change, status: :added, file: 'sub2/stewards.yml', additions: 1),
-                                                            build(:file_change, status: :removed, file: 'sub3/stewards.yml', deletions: 1)
-                                                          ])
-
-    expected_sub_stewards_changes_view = build(:steward_changes_view,
-                                               ref:           'branch_head',
-                                               stewards_file: 'sub/stewards.yml',
-                                               changes:       [
-                                                                build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1),
-                                                                build(:file_change, status: :modified, file: 'sub/stewards.yml', additions: 1)
-                                                              ])
+    expected_stewards_changes_view = {
+      rules:   Ladle::StewardRules.new(ref:           'base_head',
+                                       stewards_file: 'stewards.yml',
+      ),
+      changes: [
+                 build(:file_change, status: :removed, file: 'stewards.yml', deletions: 1),
+                 build(:file_change, status: :added, file: 'one.rb', additions: 1),
+                 build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1),
+                 build(:file_change, status: :modified, file: 'sub/stewards.yml', additions: 1),
+                 build(:file_change, status: :removed, file: 'sub2/sandwich', deletions: 1),
+                 build(:file_change, status: :added, file: 'sub2/stewards.yml', additions: 1),
+                 build(:file_change, status: :removed, file: 'sub3/stewards.yml', deletions: 1)
+               ]
+    }
+
+    expected_branch_sub_stewards_changes_view = {
+      rules:   Ladle::StewardRules.new(ref:           'branch_head',
+                                       stewards_file: 'sub/stewards.yml',
+      ),
+      changes: [
+                 build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1),
+                 build(:file_change, status: :modified, file: 'sub/stewards.yml', additions: 1)
+               ]
+    }
+
+    expected_base_sub_stewards_changes_view = {
+      rules:   Ladle::StewardRules.new(ref:           'base_head',
+                                       stewards_file: 'sub/stewards.yml',
+      ),
+      changes: [
+                 build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1),
+                 build(:file_change, status: :modified, file: 'sub/stewards.yml', additions: 1)
+               ]
+    }
 
-    expected_sub2_stewards_changes_view = build(:steward_changes_view,
-                                                ref:           'branch_head',
-                                                stewards_file: 'sub2/stewards.yml',
-                                                changes:       [
-                                                                 build(:file_change, status: :removed, file: 'sub2/sandwich', deletions: 1),
-                                                                 build(:file_change, status: :added, file: 'sub2/stewards.yml', additions: 1)
-                                                               ])
-
-    expected_sub3_stewards_changes_view = build(:steward_changes_view,
-                                                ref:           'base_head',
-                                                stewards_file: 'sub3/stewards.yml',
-                                                changes:       [
-                                                                 build(:file_change, status: :removed, file: 'sub3/stewards.yml', deletions: 1)
-                                                               ])
+    expected_sub2_stewards_changes_view = {
+      rules:   Ladle::StewardRules.new(ref:           'branch_head',
+                                       stewards_file: 'sub2/stewards.yml',
+      ),
+      changes: [
+                 build(:file_change, status: :removed, file: 'sub2/sandwich', deletions: 1),
+                 build(:file_change, status: :added, file: 'sub2/stewards.yml', additions: 1)
+
+               ]
+    }
+
+    expected_sub3_stewards_changes_view = {
+      rules:   Ladle::StewardRules.new(ref:           'base_head',
+                                       stewards_file: 'sub3/stewards.yml',
+      ),
+      changes: [
+                 build(:file_change, status: :removed, file: 'sub3/stewards.yml', deletions: 1)
+               ]
+    }
 
     notifier = mock
     notifier.expects(:notify)
       .with(deep_hash({
-              'xanderstrike'      => {
-                'sub/stewards.yml'  => [expected_sub_stewards_changes_view],
-                'stewards.yml'      => [expected_stewards_changes_view],
-                'sub3/stewards.yml' => [expected_sub3_stewards_changes_view],
-              },
-              'fadsfadsfadsfadsf' => {
-                'sub/stewards.yml' => [expected_sub_stewards_changes_view]
-              },
-              'bob'               => {
-                'stewards.yml'      => [expected_stewards_changes_view],
-                'sub3/stewards.yml' => [expected_sub3_stewards_changes_view]
-              },
-              'jeb'               => {
-                'sub/stewards.yml' => [build(:steward_changes_view,
-                                             ref:           'base_head',
-                                             stewards_file: 'sub/stewards.yml',
-                                             changes:       [
-                                                              build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1),
-                                                              build(:file_change, status: :modified, file: 'sub/stewards.yml', additions: 1)
-                                                            ])]
-              },
-              'hamburglar'        => {
-                'sub2/stewards.yml' => [expected_sub2_stewards_changes_view]
-              },
-            }))
+                        'xanderstrike'      => build(:changes_view,
+                                                     changes: [
+                                                                expected_stewards_changes_view,
+                                                                expected_branch_sub_stewards_changes_view,
+                                                                expected_sub3_stewards_changes_view
+                                                              ]),
+                        'fadsfadsfadsfadsf' => build(:changes_view,
+                                                     changes: [
+                                                                expected_branch_sub_stewards_changes_view,
+                                                              ]),
+                        'bob'               => build(:changes_view,
+                                                     changes: [
+                                                                expected_stewards_changes_view,
+                                                                expected_sub3_stewards_changes_view
+                                                              ]),
+                        'jeb'               => build(:changes_view,
+                                                     changes: [
+                                                                expected_base_sub_stewards_changes_view,
+                                                              ]),
+                        'hamburglar'        => build(:changes_view,
+                                                     changes: [
+                                                                expected_sub2_stewards_changes_view
+                                                              ])
+                      }))
 
     Ladle::PullHandler.new(client, notifier).handle(@pull_request)
   end
@@ -326,31 +358,37 @@ class PullHandlerTest < ActiveSupport::TestCase
       YAML
     end
 
-    expected_stewards_changes_view = build(:steward_changes_view,
-                                           ref:           'base_head',
-                                           stewards_file: 'hello/stewards.yml',
-                                           changes:       [
-                                                            build(:file_change, status: :added, file: 'hello/kitty/what/che.txt', additions: 1),
-                                                            build(:file_change, status: :removed, file: 'hello/kitty/what/is/stewards.yml', deletions: 1),
-                                                          ])
-
-    expected_sub_stewards_changes_view = build(:steward_changes_view,
-                                               ref:           'base_head',
-                                               stewards_file: 'hello/kitty/what/is/stewards.yml',
-                                               changes:       [
-                                                                build(:file_change, status: :removed, file: 'hello/kitty/what/is/stewards.yml', deletions: 1),
-                                                              ])
+    expected_stewards_changes_view = {
+      rules:   Ladle::StewardRules.new(ref:           'base_head',
+                                       stewards_file: 'hello/stewards.yml',
+      ),
+      changes: [
+                 build(:file_change, status: :added, file: 'hello/kitty/what/che.txt', additions: 1),
+                 build(:file_change, status: :removed, file: 'hello/kitty/what/is/stewards.yml', deletions: 1),
+               ]
+    }
+
+    expected_sub_stewards_changes_view = {
+      rules:   Ladle::StewardRules.new(ref:           'base_head',
+                                       stewards_file: 'hello/kitty/what/is/stewards.yml',
+      ),
+      changes: [
+                 build(:file_change, status: :removed, file: 'hello/kitty/what/is/stewards.yml', deletions: 1),
+               ]
+    }
 
     notifier = mock
     notifier.expects(:notify)
       .with(deep_hash({
-              'xanderstrike' => {
-                'hello/stewards.yml' => [expected_stewards_changes_view],
-                'hello/kitty/what/is/stewards.yml' => [expected_sub_stewards_changes_view]
-              },
-              'bleh'         => {
-                'hello/kitty/what/is/stewards.yml' => [expected_sub_stewards_changes_view]
-              }
+              'xanderstrike' => build(:changes_view,
+                                      changes: [
+                                                 expected_stewards_changes_view,
+                                                 expected_sub_stewards_changes_view
+                                               ]),
+              'bleh'         => build(:changes_view,
+                                      changes: [
+                                                 expected_sub_stewards_changes_view
+                                               ]),
             }))
 
     Ladle::PullHandler.new(client, notifier).handle(@pull_request)
@@ -375,15 +413,17 @@ class PullHandlerTest < ActiveSupport::TestCase
       YAML
     end
 
-    expected_stewards_changes_view = {
-      'stewards.yml' => [build(:steward_changes_view,
-                               ref:           'base_head',
-                               stewards_file: 'stewards.yml',
-                               changes:       [
-                                                build(:file_change, status: :added, file: 'one.rb', additions: 1),
-                                                build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1, deletions: 1),
-                                              ])]
-    }
+    expected_stewards_changes_view = build(:changes_view,
+                                           changes: [
+                                                      {
+                                                        rules:   Ladle::StewardRules.new(ref:           'base_head',
+                                                                                         stewards_file: 'stewards.yml'),
+                                                        changes: [
+                                                                   build(:file_change, status: :added, file: 'one.rb', additions: 1),
+                                                                   build(:file_change, status: :modified, file: 'sub/marine.rb', additions: 1, deletions: 1),
+                                                                 ]
+                                                      }
+                                                    ])
 
     notifier = mock
     notifier.expects(:notify)
@@ -415,22 +455,24 @@ class PullHandlerTest < ActiveSupport::TestCase
     notifier.stubs(:id).returns(1)
     notifier.expects(:notify)
       .with(deep_hash({
-                        'xanderstrike' => {
-                          'hello/stewards.yml' => [build(:steward_changes_view,
-                                                         ref:           'base_head',
-                                                         stewards_file: 'hello/stewards.yml',
-                                                         changes:       [
-                                                                          build(:file_change, status: :added, file: 'hello/kitty/what/is/your/favorite_food.yml', additions: 1),
-                                                                          build(:file_change, status: :added, file: 'hello/kitty/what/is/your/name.txt', additions: 1)
-                                                                        ])],
-                        },
+                        'xanderstrike' => build(:changes_view,
+                                                changes: [
+                                                           {
+                                                             rules:   Ladle::StewardRules.new(ref:           'base_head',
+                                                                                              stewards_file: 'hello/stewards.yml'),
+                                                             changes: [
+                                                                        build(:file_change, status: :added, file: 'hello/kitty/what/is/your/favorite_food.yml', additions: 1),
+                                                                        build(:file_change, status: :added, file: 'hello/kitty/what/is/your/name.txt', additions: 1)
+                                                                      ]
+                                                           }
+                                                         ])
                       }))
 
     Ladle::PullHandler.new(client, notifier).handle(@pull_request)
   end
 
   test "collect_changes" do
-    tree = Ladle::StewardTree.new('xanderstrike')
+    tree = Ladle::StewardTree.new
 
     tree.add_rules(Ladle::StewardRules.new(ref:           'base',
                                            stewards_file: 'stewards.yml',
@@ -462,34 +504,39 @@ class PullHandlerTest < ActiveSupport::TestCase
     handler = Ladle::PullHandler.new(mock('client'), mock('notifier'))
     resolved_stewards_registry = handler.send(:collect_changes, stewards_trees, changed_files)
 
-    expected_changes_view = build(:steward_changes_view,
-      stewards_file: 'stewards.yml',
-      changes:       [
-                       build(:file_change, file: 'stewards.yml'),
-                       build(:file_change, file: 'one.rb'),
-                       build(:file_change, file: 'sub/marine.rb'),
-                       build(:file_change, file: 'sub/stewards.yml'),
-                       build(:file_change, file: 'sub2/sandwich'),
-                       build(:file_change, file: 'sub3/stewards.yml')
-                     ])
-
-    assert_equal [expected_changes_view], resolved_stewards_registry['xanderstrike']['stewards.yml']
-
-    expected_changes_view = build(:steward_changes_view,
-      stewards_file: 'sub/stewards.yml',
-      changes:       [
-                       build(:file_change, file: 'sub/marine.rb'),
-                       build(:file_change, file: 'sub/stewards.yml'),
-                     ])
-
-    assert_equal [expected_changes_view], resolved_stewards_registry['xanderstrike']['sub/stewards.yml']
-
-    expected_changes_view = build(:steward_changes_view,
-      stewards_file: 'sub3/stewards.yml',
-      changes:       [
-                       build(:file_change, file: 'sub3/stewards.yml')
-                     ])
-
-    assert_equal [expected_changes_view], resolved_stewards_registry['xanderstrike']['sub3/stewards.yml']
+    expected_changes_view = build(:changes_view,
+                                  changes: [
+                                             {
+                                               rules:   Ladle::StewardRules.new(ref:           'base',
+                                                                                stewards_file: 'stewards.yml'),
+                                               changes: [
+                                                          build(:file_change, file: 'stewards.yml'),
+                                                          build(:file_change, file: 'one.rb'),
+                                                          build(:file_change, file: 'sub/marine.rb'),
+                                                          build(:file_change, file: 'sub/stewards.yml'),
+                                                          build(:file_change, file: 'sub2/sandwich'),
+                                                          build(:file_change, file: 'sub3/stewards.yml')
+                                                        ]
+                                             },
+                                             {
+                                               rules:   Ladle::StewardRules.new(ref:           'base',
+                                                                                stewards_file: 'sub/stewards.yml'),
+                                               changes: [
+                                                          build(:file_change, file: 'sub/marine.rb'),
+                                                          build(:file_change, file: 'sub/stewards.yml'),
+                                                        ]
+                                             },
+                                             {
+                                               rules:   Ladle::StewardRules.new(ref:           'base',
+                                                                                stewards_file: 'sub3/stewards.yml'),
+                                               changes: [
+                                                          build(:file_change, file: 'sub3/stewards.yml')
+                                                        ]
+                                             },
+                                           ])
+
+
+
+    assert_equal expected_changes_view, resolved_stewards_registry['xanderstrike']
   end
 end
diff --git a/test/ladle/steward_changes_view_test.rb b/test/ladle/steward_changes_view_test.rb
deleted file mode 100644
index c4d9904..0000000
--- a/test/ladle/steward_changes_view_test.rb
+++ /dev/null
@@ -1,23 +0,0 @@
-require 'test_helper'
-
-class StewardChangesViewTest < ActiveSupport::TestCase
-
-  test 'value object' do
-    changes_view1 = build(:steward_changes_view, changes: [
-                                                            build(:file_change, status: :modified, file: "app/modified_file.rb"),
-                                                            build(:file_change, status: :added, file: "app/new_file.rb"),
-                                                            build(:file_change, status: :removed, file: "app/removed_file.rb"),
-                                                          ])
-
-    changes_view2 = build(:steward_changes_view, changes: [
-                                                            build(:file_change, status: :removed, file: "app/removed_file.rb"),
-                                                            build(:file_change, status: :added, file: "app/new_file.rb"),
-                                                            build(:file_change, status: :modified, file: "app/modified_file.rb"),
-                                                          ])
-
-    assert_equal changes_view1, changes_view2
-    assert changes_view1.eql?(changes_view2)
-    assert changes_view2.eql?(changes_view1)
-    assert changes_view1.hash, changes_view2.hash
-  end
-end
diff --git a/test/ladle/steward_notifier_test.rb b/test/ladle/steward_notifier_test.rb
index 723fa9a..5b5eb0b 100644
--- a/test/ladle/steward_notifier_test.rb
+++ b/test/ladle/steward_notifier_test.rb
@@ -6,9 +6,9 @@
 class StewardNotifierTest < ActionController::TestCase
   setup do
     @steward_changes_views = {
-      'xanderstrike' => Ladle::TestData.create_steward_changes_views,
-      'counterstrike'=> Ladle::TestData.create_steward_changes_views,
-      'boop'         => Ladle::TestData.create_steward_changes_views
+      'xanderstrike' => Ladle::TestData.create_changes_view,
+      'counterstrike'=> Ladle::TestData.create_changes_view,
+      'boop'         => Ladle::TestData.create_changes_view
     }
     @pull_request = create(:pull_request, html_url: 'https://github.com/XanderStrike/test/pull/11')
     @notifier = Ladle::StewardNotifier.new('XanderStrike/test', @pull_request)
diff --git a/test/ladle/steward_tree_test.rb b/test/ladle/steward_tree_test.rb
index f85f0fd..a782b29 100644
--- a/test/ladle/steward_tree_test.rb
+++ b/test/ladle/steward_tree_test.rb
@@ -14,7 +14,7 @@ class StewardTreeTest < ActiveSupport::TestCase
     changed_files.add_file_change(build(:file_change, file: 'sub2/sandwich'))
     changed_files.add_file_change(build(:file_change, file: 'sub3/stewards.yml'))
 
-    tree = Ladle::StewardTree.new('xanderstrike')
+    tree = Ladle::StewardTree.new
 
     sub_rules = Ladle::StewardRules.new(ref:           'base',
                                         stewards_file: 'sub/stewards.yml')
diff --git a/test/mailers/previews/user_mailer_preview.rb b/test/mailers/previews/user_mailer_preview.rb
index 58c05d3..00579db 100644
--- a/test/mailers/previews/user_mailer_preview.rb
+++ b/test/mailers/previews/user_mailer_preview.rb
@@ -13,7 +13,7 @@ def notify
                         'Initial commit',
                         "We made these changes because things needed to be changed and we noticed and we had the means to make the changes, so we did."
                       ),
-                      stewards_file_map: Ladle::TestData.create_steward_changes_views)
+                      changes_view: Ladle::TestData.create_changes_view)
   end
 
   def notify_without_description
@@ -24,7 +24,7 @@ def notify_without_description
                         'Initial commit',
                         nil
                       ),
-                      stewards_file_map: Ladle::TestData.create_steward_changes_views)
+                      changes_view: Ladle::TestData.create_changes_view)
 
   end
 end
diff --git a/test/mailers/user_mailer_test.rb b/test/mailers/user_mailer_test.rb
index 5339ab7..78b6197 100644
--- a/test/mailers/user_mailer_test.rb
+++ b/test/mailers/user_mailer_test.rb
@@ -14,7 +14,7 @@ class UserMailerTest < ActionMailer::TestCase
                                 nil,
                                 nil
                               ),
-                              stewards_file_map: Ladle::TestData.create_steward_changes_views).deliver_now
+                              changes_view: Ladle::TestData.create_changes_view).deliver_now
 
     assert_not ActionMailer::Base.deliveries.empty?
 
@@ -33,7 +33,7 @@ class UserMailerTest < ActionMailer::TestCase
                                 'Hey ho!',
                                 'These changes are luminous',
                               ),
-                              stewards_file_map: Ladle::TestData.create_steward_changes_views).deliver_now
+                              changes_view: Ladle::TestData.create_changes_view).deliver_now
 
     assert_not ActionMailer::Base.deliveries.empty?