Should look for stewards files in target branch as well as source branch

[dtognazzini]

[dtognazzini]

Currently, Ladle looks for stewards files in two places:

1. Within the tree of the PR's source's head (head[:sha]).
2. Within the tree of the PR's target's head (base[:sha]).

For 2., only stewards files that have been modified or removed in the source branch's head are looked for. This was due to a misunderstanding of how the PR's target's head was presented. I thought base[:sha] represented an ancestor commit common to both the source branch's head and the target branch's head. For example:

a <--- b <---- target
^
|
 `--- c <---- branch

I thought base[:sha] would be a. Turns out, base[:sha] is b.

The idea of looking at a was that we wanted to also notify stewards that were removed as part of the branch (c); that is, a stewards file was modified or removed between the ancestor commit and the branch's head.

As far as I can tell, pushes to the target branch after the PR is created do not change the value of base[:sha]. That is, given this:

a <--- b <--- d <---- target
^
|
 `--- c <---- branch

...base[:sha] still specifies b.

The fix here, then is to remove the special handling of looking for only modified/removed stewards files within base[:sha] and instead look for any of them.

[dtognazzini]

Since we'll now look at two different trees, merging those trees into a consistent view is not trivial.

Here are some cases:

1. A stewards file exists in both branches.
   1. A single steward is only in one version.
   2. A single steward is in both versions.
      1. The steward's file filter is the same in both versions.
      2. The steward's file filter is not the same in both versions.
2. A stewards file exists in the source branch but not the target branch.
3. A stewards file exists in the target branch but not the source branch.

We could apply various rules about each of these cases. But, instead, I'm going with the shotgun approach of just running all the stewards files individually without incorporating other stewards files.

For any single steward, this will result in the following cases:

1. A file change is matched by multiple stewards files in one of the versions.
2. A file change is matched by multiple stewards files across versions.
3. A file change is not matched by any stewards files across any versions.

We could try to group these changes to reduce duplication. But, instead, I'm going with the shotgun approach of just showing all the matches individually with one exception: I'll collapse the files when the contents of the stewards file is the exact same across both versions. This may result in duplicates, but oh well.

The notification email will continue to group file changes under the stewards file that targets them and will sort those groups according to the stewards file path, preferring the source branch to break ties.

An example follows.

Given any single steward and the following file changes in the source branch (sourceBranch):

  A lib/closet/bottom_shelf/new_file.rb (1)  
  A lib/closet/top_shelf/new_file.rb (1)  
  M sub_dir/modified_file.rb (2)
  A sub_dir/new_file.rb (3)
  D sub_dir/removed_file.rb (1)
  M sub_dir/stewards.yml (3)

...and given the following stewards files in the sourceBranch grouped with the file changes they target:

[sourceBranch]/stewards.yml targets:
  M sub_dir/stewards.yml (3)

[sourceBranch]/lib/closet/stewards.yml targets:
  A lib/closet/top_shelf/new_file.rb (1)

[sourceBranch]/sub_dir/stewards.yml targets:
  M sub_dir/modified_file.rb (2)
  A sub_dir/new_file.rb (3)
  D sub_dir/removed_file.rb (1)
  M sub_dir/stewards.yml (3)

...and given the following stewards files in the targetBranch grouped with the file changes they target:

[targetBranch]/stewards.yml targets:
  M sub_dir/stewards.yml (3)

[targetBranch]/lib/closet/stewards.yml targets:
  A lib/closet/top_shelf/new_file.rb (1) 
  A lib/closet/bottom_shelf/new_file.rb (1) 

[targetBranch]/lib/closet/bottom_shelf/stewards.yml targets:
  A lib/closet/bottom_shelf/new_file.rb (1) 

...and given the following stewards file changes:

/stewards.yml
no changes

/lib/closet/stewards.yml
version in both branches includes subject steward, targetBranch branch version targets more files.

/lib/closet/bottom_shelf/stewards.yml
only exists in targetBranch

/sub_dir/stewards.yml
changed in sourceBranch to include subject steward

...the following email will be sent:

<preamble>

Changes monitored by [sourceBranch]/stewards.yml:

  M sub_dir/stewards.yml (3)

Changes monitored by [sourceBranch]/lib/closet/stewards.yml:

  A lib/closet/top_shelf/new_file.rb (1)

Changes monitored by [targetBranch]/lib/closet/stewards.yml:

  A lib/closet/top_shelf/new_file.rb (1) 
  A lib/closet/bottom_shelf/new_file.rb (1) 

Changes monitored by [targetBranch]/lib/closet/bottom_shelf/stewards.yml:

  A lib/closet/bottom_shelf/new_file.rb (1) 

Changes monitored by [sourceBranch]/sub_dir/stewards.yml:

  M sub_dir/modified_file.rb (2)
  A sub_dir/new_file.rb (3)
  D sub_dir/removed_file.rb (1)
  M sub_dir/stewards.yml (3)

<postamble>

Notes:

[sourceBranch]/stewards.yml
only listed under the sourceBranch because the contents are the same across branches.

[sourceBranch]/lib/closet/stewards.yml
[targetBranch]/lib/closet/stewards.yml
listed for both branches because the contents don't match across branches.

[targetBranch]/lib/closet/bottom_shelf/stewards.yml
listed for targetBranch because it does not exist on sourceBranch.

[sourceBranch]/sub_dir/stewards.yml
listed only for the sourceBranch because the version in the targetBranch, while different, did not match any file changes for this steward.

[dtognazzini]

We could try to group these changes to reduce duplication. But, instead, I'm going with the shotgun approach of just showing all the matches individually with one exception: I'll collapse the files when the contents of the stewards file is the exact same across both versions. This may result in duplicates, but oh well.

The downside of this is that the file could have changed for some other reason that doesn't impact the files targeted for the steward. So, instead, equivalence will be tested against the files targeted by the stewards file instead of the contents of the stewards file.