commit 15a982f84093b14bb91c5849dabc957987079339
author Edwin Kempin <ekempin@google.com> Wed Aug 07 14:43:35 2024 +0000
committer Edwin Kempin <ekempin@google.com> Mon Aug 12 09:40:06 2024 +0000
tree 329405bd093c7b0a516efbe955bb832983f04e81
parent 6181ef2e0d89811328b1857bb86f0442009bb8dd

PathCodeOwners: Flatten the return type (step 1/3)

Currently the path code owner computation returns
OptionalResultWithMessages<PathCodeOwnersResult>:

* OptionalResultWithMessages: arbitrary result (PathCodeOwnersResult in
  this case) plus messages
* PathCodeOwnersResult: the resolved CodeOwnerConfig plus
  resolved/unresolved imports (PathCodeOwners resolves a CodeOwnerConfig
  for the path for which code owners should be computed; resolving means
  that non-matching per-file code owner sets are dropped, imported code
  owner configs are loaded and replaced with the parts of them which
  should be imported, global code owner sets are dropped if any matching
  per-file code owner set has the ignoreGlobalAndParentCodeOwners flag
  set to true and that the the ignoreParentCodeOwners flag is set to
  true if any matching per-file code owner set has the
  ignoreGlobalAndParentCodeOwners flag set to true; the result is called
  a "resolved" CodeOwnerConfig)
* CodeOwnerConfig: the resolved code owner config representing the
  actual result of the path code owner resolution (relevant code owner
  sets and the ignoreParentCodeOwners flag)

This unnecessarily complicated return type is the result of extending
the path code owners computation twice. Initially we just returned the
resolved CodeOwnerConfig, but then we added OptionalResultWithMessages
to return debug messages and then PathCodeOwnersResult to return
resolved/unresolved imports.

To clean this up we want to include all return data in
PathCodeOwnersResult and then just return PathCodeOwnersResult.

To make reviewing this refactoring easier, we do this cleanup in
multiple steps

1. [this change] Drop usage of CodeOwnerConfig to return the relevant
   code owner sets and the ignoreParentCodeOwners flag:

Add fields for the code owner sets and the ignoreParentCodeOwners flag
directly to PathCodeOwnersResult. The other fields from CodeOwnerConfig
are not used and also users of PathCodeOwnersResult never access the
wrapped CodeOwnerConfig directly.

Note, temporarily this adds more boiler-plate code for local vars that
collect the result data, but we will clean this up in a later step (see
step 3. below).

2. [follow-up change] Drop usage of OptionalResultWithMessages:

Using OptionalResultWithMessages doesn't make sense since the path code
owner computation always returns a result (i.e. the result is not
optional).

Instead include the debug messages directly into PathCodeOwnersResult.

3. [follow-up change] Add a AutoValue builder for PathCodeOwnersResult:

This allows us to get rid of local vars to collect the result data.

Having an AutoValue builder for PathCodeOwnersResult also allows us to
unify how results within PathCodeOwners are passed between methods. At
the moment the resolveImports method has a return value to return
resolved/unresolved imports and the messages, and in addition it gets a
CodeOwnerConfig.Builder passed in to write out relevant parts of the
imported CodeOwnerConfig. Using 2 different approaches to return results
is confusing. If we have an AutoValue builder for PathCodeOwnersResult
we can just use this builder to collect all the results.

Change-Id: I304e66a4eee29d79c490ba1c6013c8a1f59338ed
Signed-off-by: Edwin Kempin <ekempin@google.com>