dmitri.shuralyov.com/service/change/...

Implement Prev/Next Commit buttons.

Remove ListCommitsOptions from ListCommits. Expected usage is to always
get all commits, then find the one you're interested in. This is what's
needed to implement prev/next commit buttons, and more closely matches
what's returned by 3rd party APIs.

The button styling can be improved; to be done later.
dmitshur committed 2 years ago commit 241d5db0228789eb9d6982990c98f20dc3839eef
changes.go
@@ -17,11 +17,11 @@ type Service interface {
 	Count(ctx context.Context, repo string, opt ListOptions) (uint64, error)
 
 	// Get a change.
 	Get(ctx context.Context, repo string, id uint64) (Change, error)
 	// ListCommits lists change commits.
-	ListCommits(ctx context.Context, repo string, id uint64, opt *ListCommitsOptions) ([]Commit, error)
+	ListCommits(ctx context.Context, repo string, id uint64) ([]Commit, error)
 	// Get a change diff.
 	GetDiff(ctx context.Context, repo string, id uint64, opt *ListCommitsOptions) ([]byte, error)
 
 	// ListComments lists comments for specified change id.
 	ListComments(ctx context.Context, repo string, id uint64, opt *ListCommentsOptions) ([]issues.Comment, error)
gerritapi/gerritapi.go
@@ -124,47 +124,32 @@ func state(status string) changes.State {
 	default:
 		panic("unreachable")
 	}
 }
 
-func (s service) ListCommits(ctx context.Context, _ string, id uint64, opt *changes.ListCommitsOptions) ([]changes.Commit, error) {
+func (s service) ListCommits(ctx context.Context, _ string, id uint64) ([]changes.Commit, error) {
 	change, _, err := s.cl.Changes.GetChange(fmt.Sprint(id), &gerrit.ChangeOptions{
 		AdditionalFields: []string{"DETAILED_ACCOUNTS", "ALL_REVISIONS"},
 		//AdditionalFields: []string{"ALL_REVISIONS", "ALL_COMMITS"}, // TODO: Consider using git committer/author instead...
 	})
 	if err != nil {
 		return nil, err
 	}
 	if change.Status == "DRAFT" {
 		return nil, os.ErrNotExist
 	}
-	switch opt {
-	case nil:
-		commits := make([]changes.Commit, len(change.Revisions))
-		for sha, r := range change.Revisions {
-			commits[r.Number-1] = changes.Commit{
-				SHA:     sha,
-				Message: fmt.Sprintf("Patch Set %d", r.Number),
-				// TODO: r.Uploader and r.Created describe the committer, not author.
-				Author:     s.gerritUser(r.Uploader),
-				AuthorTime: time.Time(r.Created),
-			}
-		}
-		return commits, nil
-	default:
-		r, ok := change.Revisions[opt.Commit]
-		if !ok {
-			return nil, os.ErrNotExist
-		}
-		return []changes.Commit{{
-			SHA:     opt.Commit,
+	commits := make([]changes.Commit, len(change.Revisions))
+	for sha, r := range change.Revisions {
+		commits[r.Number-1] = changes.Commit{
+			SHA:     sha,
 			Message: fmt.Sprintf("Patch Set %d", r.Number),
 			// TODO: r.Uploader and r.Created describe the committer, not author.
 			Author:     s.gerritUser(r.Uploader),
 			AuthorTime: time.Time(r.Created),
-		}}, nil
+		}
 	}
+	return commits, nil
 }
 
 func (s service) GetDiff(ctx context.Context, _ string, id uint64, opt *changes.ListCommitsOptions) ([]byte, error) {
 	switch opt {
 	case nil:
githubapi/githubapi.go
@@ -207,44 +207,30 @@ func (s service) Get(ctx context.Context, rs string, id uint64) (changes.Change,
 		CreatedAt: pr.CreatedAt.Time,
 		Commits:   pr.Commits.TotalCount,
 	}, nil
 }
 
-func (s service) ListCommits(ctx context.Context, rs string, id uint64, opt *changes.ListCommitsOptions) ([]changes.Commit, error) {
+func (s service) ListCommits(ctx context.Context, rs string, id uint64) ([]changes.Commit, error) {
 	repo, err := ghRepoSpec(rs)
 	if err != nil {
 		// TODO: Map to 400 Bad Request HTTP error.
 		return nil, err
 	}
-	switch opt {
-	case nil:
-		cs, _, err := s.clV3.PullRequests.ListCommits(ctx, repo.Owner, repo.Repo, int(id), nil)
-		if err != nil {
-			return nil, err
-		}
-		var commits []changes.Commit
-		for _, c := range cs {
-			commits = append(commits, changes.Commit{
-				SHA:        *c.SHA,
-				Message:    *c.Commit.Message,
-				Author:     ghUser(c.Author),
-				AuthorTime: *c.Commit.Author.Date,
-			})
-		}
-		return commits, nil
-	default:
-		c, _, err := s.clV3.Repositories.GetCommit(ctx, repo.Owner, repo.Repo, opt.Commit)
-		if err != nil {
-			return nil, err
-		}
-		return []changes.Commit{{
+	cs, _, err := s.clV3.PullRequests.ListCommits(ctx, repo.Owner, repo.Repo, int(id), nil)
+	if err != nil {
+		return nil, err
+	}
+	var commits []changes.Commit
+	for _, c := range cs {
+		commits = append(commits, changes.Commit{
 			SHA:        *c.SHA,
 			Message:    *c.Commit.Message,
 			Author:     ghUser(c.Author),
 			AuthorTime: *c.Commit.Author.Date,
-		}}, nil
+		})
 	}
+	return commits, nil
 }
 
 func (s service) GetDiff(ctx context.Context, rs string, id uint64, opt *changes.ListCommitsOptions) ([]byte, error) {
 	repo, err := ghRepoSpec(rs)
 	if err != nil {
maintner/maintner.go
@@ -104,11 +104,11 @@ func (s service) Count(_ context.Context, repo string, opt changes.ListOptions)
 func (s service) Get(ctx context.Context, _ string, id uint64) (changes.Change, error) {
 	// TODO.
 	return changes.Change{}, fmt.Errorf("Get: not implemented")
 }
 
-func (s service) ListCommits(ctx context.Context, _ string, id uint64, opt *changes.ListCommitsOptions) ([]changes.Commit, error) {
+func (s service) ListCommits(ctx context.Context, _ string, id uint64) ([]changes.Commit, error) {
 	return nil, fmt.Errorf("ListCommits: not implemented")
 }
 
 func (s service) GetDiff(ctx context.Context, _ string, id uint64, opt *changes.ListCommitsOptions) ([]byte, error) {
 	// TODO.