~robertm: 1 Implement "hut hg update <repo> --readme <file>" 4 files changed, 86 insertions(+), 2 deletions(-)
The missing context is knowledge of how "hut git update <repo> --readme <file>" works. People who are in the target audience of the patch have that knowledge. Hint: Every line of source code in the patch is derived from a corresponding line of "hut git update" code. You can compare each block of lines with its hut-git counterpart, to see the differences. Another hint: Files named "gql.go" are not source code, they are generator output.
Copy & paste the following snippet into your terminal to import this patchset into git:
curl -s https://lists.sr.ht/~emersion/hut-dev/patches/50961/mbox | git am -3Learn more about email & git
From: Robert Munyer <robertm@git.sr.ht> --- doc/hut.1.scd | 10 ++++++ hg.go | 58 ++++++++++++++++++++++++++++++++++ srht/hgsrht/gql.go | 12 ++++++- srht/hgsrht/operations.graphql | 8 ++++- 4 files changed, 86 insertions(+), 2 deletions(-) diff --git a/doc/hut.1.scd b/doc/hut.1.scd index 76d1291..c6e4c1d 100644 --- a/doc/hut.1.scd +++ b/doc/hut.1.scd @@ -321,6 +321,16 @@ Options are: *list* [owner] List repositories. +*update* <repo> [options...] + Update a repository. + + Options are: + + *--readme* <file> + Update the custom README settings. You can read the HTML from a file or + pass "-" as the filename to read from _stdin_. + To clear the custom README use an empty string "". + *user-webhook create* [options...] Create a user webhook. When this command is run from a terminal, the query will be read from _$EDITOR_, otherwise it defaults to stdin. diff --git a/hg.go b/hg.go index 1112232..3548c3c 100644 --- a/hg.go +++ b/hg.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "log" + "os" "strings" "git.sr.ht/~emersion/hut/srht/hgsrht" @@ -20,6 +21,7 @@ func newHgCommand() *cobra.Command { cmd.AddCommand(newHgListCommand()) cmd.AddCommand(newHgCreateCommand()) cmd.AddCommand(newHgDeleteCommand()) + cmd.AddCommand(newHgUpdateCommand()) cmd.AddCommand(newHgUserWebhookCommand()) return cmd } @@ -145,6 +147,62 @@ func newHgDeleteCommand() *cobra.Command { return cmd } +func newHgUpdateCommand() *cobra.Command { + var readme string + run := func(cmd *cobra.Command, args []string) { + ctx := cmd.Context() + + name, owner, instance := parseResourceName(args[0]) + + c := createClientWithInstance("hg", cmd, instance) + id := getHgRepoID(c, ctx, name, owner) + + var input hgsrht.RepoInput + + if readme == "" && cmd.Flags().Changed("readme") { + _, err := hgsrht.ClearCustomReadme(c.Client, ctx, id) + if err != nil { + log.Fatalf("failed to unset custom README: %v", err) + } + } else if readme != "" { + var ( + b []byte + err error + ) + + if readme == "-" { + b, err = io.ReadAll(os.Stdin) + } else { + b, err = os.ReadFile(readme) + } + if err != nil { + log.Fatalf("failed to read custom README: %v", err) + } + + s := string(b) + input.Readme = &s + } + + repo, err := hgsrht.UpdateRepository(c.Client, ctx, id, input) + if err != nil { + log.Fatal(err) + } else if repo == nil { + log.Fatalf("failed to update repository %q", name) + } + + log.Printf("Successfully updated repository %q\n", repo.Name) + } + cmd := &cobra.Command{ + Use: "update <repo>", + Short: "Update a repository", + Args: cobra.ExactArgs(1), + ValidArgsFunction: cobra.NoFileCompletions, + Run: run, + } + cmd.Flags().StringVar(&readme, "readme", "", "update the custom README") + return cmd +} + func newHgUserWebhookCommand() *cobra.Command { cmd := &cobra.Command{ Use: "user-webhook", diff --git a/srht/hgsrht/gql.go b/srht/hgsrht/gql.go index 533d031..729d26d 100644 --- a/srht/hgsrht/gql.go +++ b/srht/hgsrht/gql.go @@ -483,7 +483,7 @@ func CreateRepository(client *gqlclient.Client, ctx context.Context, name string } func UpdateRepository(client *gqlclient.Client, ctx context.Context, id int32, input RepoInput) (updateRepository *Repository, err error) { - op := gqlclient.NewOperation("mutation updateRepository ($id: Int!, $input: RepoInput!) {\n\tupdateRepository(id: $id, input: $input) {\n\t\tid\n\t}\n}\n") + op := gqlclient.NewOperation("mutation updateRepository ($id: Int!, $input: RepoInput!) {\n\tupdateRepository(id: $id, input: $input) {\n\t\tname\n\t}\n}\n") op.Var("id", id) op.Var("input", input) var respData struct { @@ -493,6 +493,16 @@ func UpdateRepository(client *gqlclient.Client, ctx context.Context, id int32, i return respData.UpdateRepository, err } +func ClearCustomReadme(client *gqlclient.Client, ctx context.Context, id int32) (updateRepository *Repository, err error) { + op := gqlclient.NewOperation("mutation clearCustomReadme ($id: Int!) {\n\tupdateRepository(id: $id, input: {readme:null}) {\n\t\tname\n\t}\n}\n") + op.Var("id", id) + var respData struct { + UpdateRepository *Repository + } + err = client.Execute(ctx, op, &respData) + return respData.UpdateRepository, err +} + func DeleteRepository(client *gqlclient.Client, ctx context.Context, id int32) (deleteRepository *Repository, err error) { op := gqlclient.NewOperation("mutation deleteRepository ($id: Int!) {\n\tdeleteRepository(id: $id) {\n\t\tname\n\t}\n}\n") op.Var("id", id) diff --git a/srht/hgsrht/operations.graphql b/srht/hgsrht/operations.graphql index d163323..00ded3d 100644 --- a/srht/hgsrht/operations.graphql +++ b/srht/hgsrht/operations.graphql @@ -98,7 +98,13 @@ mutation createRepository( mutation updateRepository($id: Int!, $input: RepoInput!) { updateRepository(id: $id, input: $input) { - id + name + } +} + +mutation clearCustomReadme($id: Int!) { + updateRepository(id: $id, input: { readme: null }) { + name } }
Gerhard Sittig <gerhard.sittig@gmx.net>Am referring to these "late parts". Cannot tell what the above first GraphQL change does, not being familiar with that either. But it was unexpected to see that change in the context of adding an "update repo" feature. And it looked a little "hidden" to me, easily overlooked. That first change also happens to change the context of the second change, which does not only introduce the README adjustment, but also touches what existed before. If two different things were going on here, then I'd suggest to first fix/adjust/improve the existing doc/API/text in a separate commit. (You can tell how I'm uncertain what the motivation is or what the consequences are.) And then have a second commit which exclusively introduces the new feature without carrying other baggage. This probably simplifies review and reasoning about what the resulting code does after the change. Also improves documentation for those who later have to read the history.The change to the "updateRepository" mutation causes it to return the name instead of the ID of the updated hg repository. This conforms to the corresponding git code, because for the "hut" user seeing the name is much more useful than seeing the ID of a repository. Until now the function was only called for hut's "export" command which throws away the return value, so no further update to old code was necessary. So all in all no important change to existing behaviour is happening.Or maybe I'm missing context (very much possible). Then the commit message could eliminate that confusion? :) virtually yours Gerhard Sittig -- If you don't understand or are scared by any of the above ask your parents or an adult to help you.
-- 2.43.0
builds.sr.ht <builds@sr.ht>hut/patches/.build.yml: SUCCESS in 30s [Implement "hut hg update <repo> --readme <file>"][0] from [~robertm][1] [0]: https://lists.sr.ht/~emersion/hut-dev/patches/50961 [1]: mailto:2587875257@munyer.com ✓ #1194351 SUCCESS hut/patches/.build.yml https://builds.sr.ht/~emersion/job/1194351
Gerhard Sittig <gerhard.sittig@gmx.net>Have to say that I'm neither familiar with mercurial nor the Go programming language nor the sr.ht implementation nor the GraphQL approach. The feedback given here is purely based on "eyeballing" the submitted change, and trying to see how it works in general and how it feels. With that out of the way: Are there two kinds of changes involved here? The commit starts out and reads as if it'd introduce support for "update repo, with optional README adjustment". The subject agrees with that. Later parts of the change then appear to also change existing behaviour? Which could be surprising. If this is the case, then should these two things (the extension, and a potential change of existing behaviour or the doc update or fix/improvement for something that existed before), go separately? Or is it "a byproduct" of having an internal "update repo" helper that has been in use before, while an "update repo" user accessible action has not existed? Or should the commit message mention when more is happening which is not included in the subject? (Which can be acceptable, just needs to become available when reading more of the message. Ideally before diving into the code change itself.)
Pushed, thanks for your patch!