-
Notifications
You must be signed in to change notification settings - Fork 7.3k
Add non-local PR template support #5097
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| m.templates = make([]Template, len(issueTemplates)) | ||
| for i := range issueTemplates { | ||
| m.templates[i] = &issueTemplates[i] | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved this work into the list functions so that they would both have the same function signatures.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking this on!
pkg/cmd/pr/shared/templates.go
Outdated
| return hasQuerySupport && hasMutationSupport, nil | ||
| } | ||
|
|
||
| func hasPullRequestTemplateSupport(httpClient *http.Client, hostname string) (bool, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function does the exact same query as hasIssueTemplateSupport, except that it scans for a different field on a Repository object. Could this lookup instead piggyback on hasIssueTemplateSupport so that fewer API calls are done?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup good call. I consolidated hasIssueTemplateSupport and hasPullRequestTemplateSupport into a single hasTemplateSupport function which uses one query and supports both issues and pull requests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! One minor note about keeping the GraphQL query unchanged (for cache key purposes)
|
Thank you, @samcoe! |
This PR adds the ability to use non-local templates when opening a PR. It is mostly a copy of the work done for issue create templates.
Closes #838