Conversation
This enforces strict separation between serialization structs used for repository creation payload with respect to whether GraphQL or REST was used. Before, a field added to a GraphQL payload would leak to REST payload (and vice versa).
|
I've pushed a refactoring that creates strictly separate payload structs for REST vs GraphQL repository creation. Before, the same Summary of changes:
Please review! |
pkg/cmd/repo/create/create.go
Outdated
| var templateRepo ghrepo.Interface | ||
| apiClient := api.NewClientFromHTTP(httpClient) | ||
|
|
||
| cloneURL := opts.Template |
There was a problem hiding this comment.
I know that this was already in place but I think it would be clearer to rename this to templateRepoURL or something similar since toClone was renamed to remplateRepo.
| Name string `json:"name"` | ||
| Visibility string `json:"visibility"` | ||
| OwnerID string `json:"ownerId,omitempty"` | ||
| type createRepositoryInput struct { |
There was a problem hiding this comment.
It isn't immediately obvious from the name what the difference between repoCreateInput, createRepositoryInputV3, and createRepositoryInput is, I think comments about their purpose could help clear that up.
| path := "user/repos" | ||
| if isOrg { | ||
| path = fmt.Sprintf("orgs/%s/repos", input.OwnerLogin) | ||
| inputv3.Visibility = strings.ToLower(input.Visibility) |
There was a problem hiding this comment.
This is not a CLI issue but the existence of private and visibility in this API endpoint is confusing.
| input := repoCreateInput{ | ||
| Description: "roasted chestnuts", | ||
| HomepageURL: "http://example.com", | ||
| func Test_repoCreate(t *testing.T) { |
There was a problem hiding this comment.
These tests are great, just for completeness I think one case that is missing is creating a repo with a hostname that is not github.com.
| } | ||
| } | ||
|
|
||
| func RESTPayload(responseStatus int, responseBody string, cb func(payload map[string]interface{})) Responder { |
Fixes #3969
Fixes #3931
Followup to #3746