dmitri.shuralyov.com/website/gido/...

refactor code that populates issues and changes

This code has gotten quite terse and hard to understand.
Refactor it, without changing behavior, so it is easier
to read and extend.

There are now two separated sections. One adds issues/changes to
issuesAndChanges. Another populates {open,closed}{Issues,Changes}.

The next commit will fix an omission.

Updates https://dmitri.shuralyov.com/website/gido/...$issues/5.
dmitshur committed 7 months ago commit 8224e12ba9aea8d30dbeb643dca121cefec05726
service.go
@@ -36,15 +36,19 @@ type Directory struct {
 	OpenChanges, ClosedChanges []change.Change
 }
 
 type IssueAndPaths struct {
 	issues.Issue
-	Paths []string // One or more import paths that are associated with the issue.
+	// Paths is one or more import paths that are associated with the issue,
+	// all of which must have a corresponding existing directory.
+	Paths []string
 }
 type ChangeAndPaths struct {
 	change.Change
-	Paths []string // One or more import paths that are associated with the change.
+	// Paths is one or more import paths that are associated with the change,
+	// all of which must have a corresponding existing directory.
+	Paths []string
 }
 
 func newService(ctx context.Context) *service {
 	issuesAndChanges := emptyDirectories()
 
@@ -157,13 +161,13 @@ func initCorpus(ctx context.Context) (*maintner.Corpus, *maintner.GitHubRepo, er
 	}
 	return corpus, repo, nil
 }
 
 func issuesAndChanges(repo *maintner.GitHubRepo, gerrit *maintner.Gerrit) (
-	issuesAndChanges map[string]*Directory,
-	openIssues, closedIssues []IssueAndPaths,
-	openChanges, closedChanges []ChangeAndPaths,
+	issuesAndChanges map[string]*Directory, // Import Path -> Existing Directory.
+	openIssues, closedIssues []IssueAndPaths, // All issues that have at least one valid path.
+	openChanges, closedChanges []ChangeAndPaths, // All changes that have at least one valid path.
 ) {
 	issuesAndChanges = emptyDirectories()
 
 	// Collect issues.
 	err := repo.ForeachIssue(func(i *maintner.GitHubIssue) error {
@@ -199,40 +203,55 @@ func issuesAndChanges(repo *maintner.GitHubRepo, gerrit *maintner.Gerrit) (
 				CreatedAt: i.Created,
 			},
 			Replies: replies,
 		}
 
+		// paths tracks import paths from issue prefix that correspond to existing directories.
+		// len(paths) == 0 means no matching directories.
 		var paths []string
+
+		// Add issue to issuesAndChanges.
 		for _, p := range pkgs {
 			ic := issuesAndChanges[p]
 			if ic == nil {
+				// p doesn't correspond to an existing directory, skip.
 				continue
 			}
+
+			// Add import path p to paths because it corresponds to an existing directory.
 			paths = append(paths, p)
+
+			// Add issue to its directory.
 			switch issue.State {
 			case issues.OpenState:
 				ic.OpenIssues = append(ic.OpenIssues, issue)
 			case issues.ClosedState:
 				ic.ClosedIssues = append(ic.ClosedIssues, issue)
 			}
 		}
-		if len(paths) > 0 {
-			issue.Title = ImportPathsToFullPrefix(paths) + title
+		if len(paths) == 0 {
+			issue.Title = i.Title
+
+			// Add issue to otherPackages because it wasn't added anywhere else.
+			ic := issuesAndChanges[otherPackages]
 			switch issue.State {
 			case issues.OpenState:
-				openIssues = append(openIssues, IssueAndPaths{issue, paths})
+				ic.OpenIssues = append(ic.OpenIssues, issue)
 			case issues.ClosedState:
-				closedIssues = append(closedIssues, IssueAndPaths{issue, paths})
+				ic.ClosedIssues = append(ic.ClosedIssues, issue)
 			}
-		} else {
-			ic := issuesAndChanges[otherPackages]
-			issue.Title = i.Title
+		}
+
+		// Populate openIssues and closedIssues.
+		if len(paths) > 0 {
+			issue.Title = ImportPathsToFullPrefix(paths) + title
+
 			switch issue.State {
 			case issues.OpenState:
-				ic.OpenIssues = append(ic.OpenIssues, issue)
+				openIssues = append(openIssues, IssueAndPaths{issue, paths})
 			case issues.ClosedState:
-				ic.ClosedIssues = append(ic.ClosedIssues, issue)
+				closedIssues = append(closedIssues, IssueAndPaths{issue, paths})
 			}
 		}
 
 		return nil
 	})
@@ -273,43 +292,58 @@ func issuesAndChanges(repo *maintner.GitHubRepo, gerrit *maintner.Gerrit) (
 				Author:    gitUser(cl.Commit.Author),
 				CreatedAt: cl.Created,
 				Replies:   len(cl.Messages),
 			}
 
+			// paths tracks import paths from change prefix that correspond to existing directories.
+			// len(paths) == 0 means no matching directories.
 			var paths []string
+
+			// Add change to issuesAndChanges.
 			for _, p := range pkgs {
 				ic := issuesAndChanges[p]
 				if ic == nil {
+					// p doesn't correspond to an existing directory, skip.
 					continue
 				}
+
+				// Add import path p to paths because it corresponds to an existing directory.
 				paths = append(paths, p)
+
+				// Add change to its directory.
 				switch c.State {
 				case change.OpenState:
 					ic.OpenChanges = append(ic.OpenChanges, c)
 				case change.ClosedState, change.MergedState:
 					ic.ClosedChanges = append(ic.ClosedChanges, c)
 				}
 			}
-			if len(paths) > 0 {
-				c.Title = ImportPathsToFullPrefix(paths) + title
+			if len(paths) == 0 {
+				c.Title = prefixedTitle
+
+				// Add change to otherPackages or root because it wasn't added anywhere else.
+				ic := issuesAndChanges[otherPackages]
+				if root != "" {
+					ic = issuesAndChanges[root]
+				}
 				switch c.State {
 				case change.OpenState:
-					openChanges = append(openChanges, ChangeAndPaths{c, paths})
+					ic.OpenChanges = append(ic.OpenChanges, c)
 				case change.ClosedState, change.MergedState:
-					closedChanges = append(closedChanges, ChangeAndPaths{c, paths})
-				}
-			} else {
-				ic := issuesAndChanges[root]
-				if ic == nil {
-					ic = issuesAndChanges[otherPackages]
+					ic.ClosedChanges = append(ic.ClosedChanges, c)
 				}
-				c.Title = prefixedTitle
+			}
+
+			// Populate openChanges and closedChanges.
+			if len(paths) > 0 {
+				c.Title = ImportPathsToFullPrefix(paths) + title
+
 				switch c.State {
 				case change.OpenState:
-					ic.OpenChanges = append(ic.OpenChanges, c)
+					openChanges = append(openChanges, ChangeAndPaths{c, paths})
 				case change.ClosedState, change.MergedState:
-					ic.ClosedChanges = append(ic.ClosedChanges, c)
+					closedChanges = append(closedChanges, ChangeAndPaths{c, paths})
 				}
 			}
 
 			return nil
 		})