From a767fd7910d28d6005972f55a1a9ce0e1e24acbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 22 Jan 2020 18:37:50 +0100 Subject: [PATCH] Add code comments for tricky parts --- api/queries_repo.go | 5 +++++ command/pr_create.go | 7 ++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/api/queries_repo.go b/api/queries_repo.go index b77dc2d34..ae07ed1e9 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -112,6 +112,9 @@ func RepoNetwork(client *Client, repos []Repo) (RepoNetworkResult, error) { `, i, repo.RepoOwner(), repo.RepoName())) } + // Since the query is constructed dynamically, we can't parse a response + // format using a static struct. Instead, hold the raw JSON data until we + // decide how to parse it manually. graphqlResult := map[string]*json.RawMessage{} result := RepoNetworkResult{} @@ -157,6 +160,8 @@ func RepoNetwork(client *Client, repos []Repo) (RepoNetworkResult, error) { // sort keys to ensure `repo_{N}` entries are processed in order sort.Sort(sort.StringSlice(keys)) + // Iterate over keys of GraphQL response data and, based on its name, + // dynamically allocate the target struct an individual message gets decoded to. for _, name := range keys { jsonMessage := graphqlResult[name] if name == "viewer" { diff --git a/command/pr_create.go b/command/pr_create.go index 4f8a2a70a..7eff43489 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -232,6 +232,8 @@ func init() { prCreateCmd.Flags().BoolP("web", "w", false, "Open the web browser to create a pull request") } +// cap the number of git remotes looked up, since the user might have an +// unusally large number of git remotes const maxRemotesForLookup = 5 func resolveRemotesToRepos(remotes context.Remotes, client *api.Client, base string) (resolvedRemotes, error) { @@ -252,6 +254,8 @@ func resolveRemotesToRepos(remotes context.Remotes, client *api.Client, base str } } if hasBaseOverride && !foundBaseOverride { + // additionally, look up the explicitly specified base repo if it's not + // already covered by git remotes repos = append(repos, baseOverride) } @@ -313,7 +317,8 @@ func (r resolvedRemotes) HeadRepo() (*api.Repository, error) { func (r resolvedRemotes) RemoteForRepo(repo api.Repo) (*context.Remote, error) { for i, remote := range r.remotes { if isSameRepo(remote, repo) || - // FIXME: express better that this is because of repo renames + // additionally, look up the resolved repository name in case this + // git remote points to this repository via a redirect (r.network.Repositories[i] != nil && isSameRepo(r.network.Repositories[i], repo)) { return remote, nil }