Clarify assignee/reviewer fetching behavior
Add clarifying comment in pkg/cmd/pr/edit/edit.go to note that missing assignee/reviewer search functions trigger a downstream fallback to legacy fetching. In pkg/cmd/pr/shared/editable.go remove a redundant line and add a TODO urging migration of non-interactive assignee updates to use the new logins input with ReplaceActorsForAssignable to avoid unnecessary fetching. These are comment and doc changes to clarify intent and future improvements.
This commit is contained in:
parent
4569aae0e4
commit
aac223ab71
2 changed files with 4 additions and 2 deletions
|
|
@ -297,7 +297,8 @@ func editRun(opts *EditOptions) error {
|
|||
apiClient := api.NewClientFromHTTP(httpClient)
|
||||
|
||||
// Wire up search functions for assignees and reviewers.
|
||||
// Only enabled on github.com.
|
||||
// When these aren't wired up, it triggers a downstream fallback
|
||||
// to legacy reviewer/assignee fetching.
|
||||
if issueFeatures.ActorIsAssignable {
|
||||
editable.AssigneeSearchFunc = assigneeSearchFunc(apiClient, repo, &editable, pr.ID)
|
||||
editable.ReviewerSearchFunc = reviewerSearchFunc(apiClient, repo, &editable, pr.ID)
|
||||
|
|
|
|||
|
|
@ -465,7 +465,6 @@ func FetchOptions(client *api.Client, repo ghrepo.Interface, editable *Editable,
|
|||
fetchAssignees := false
|
||||
if editable.Assignees.Edited {
|
||||
// Similar as above, this is likely an interactive flow if no Add/Remove slices are set.
|
||||
// The addition here is that we also check for an assignee search func.
|
||||
// If we have a search func, we don't need to fetch assignees since we
|
||||
// assume that will be done dynamically in the prompting flow.
|
||||
if len(editable.Assignees.Add) == 0 && len(editable.Assignees.Remove) == 0 && editable.AssigneeSearchFunc == nil {
|
||||
|
|
@ -473,6 +472,8 @@ func FetchOptions(client *api.Client, repo ghrepo.Interface, editable *Editable,
|
|||
}
|
||||
// However, if we have Add/Remove operations (non-interactive flow),
|
||||
// we do need to fetch the assignees.
|
||||
// TODO: KW noninteractive assignees need to migrate to directly use
|
||||
// new logins input with ReplaceActorsForAssignable to prevent fetching.
|
||||
if len(editable.Assignees.Add) > 0 || len(editable.Assignees.Remove) > 0 {
|
||||
fetchAssignees = true
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue