Skip to content

Commit

Permalink
chore: convert all todos to real bugs (#41)
Browse files Browse the repository at this point in the history
I did leave some breadcrumbs in the code still so we know where some of these changes should be made, but every todo now has a bug.
  • Loading branch information
codyoss authored Oct 30, 2024
1 parent 0840636 commit f0da672
Show file tree
Hide file tree
Showing 8 changed files with 6 additions and 23 deletions.
5 changes: 0 additions & 5 deletions generator/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,3 @@ or to playback an old input without the need for `protoc`:
```bash
go run github.com/googleapis/google-cloud-rust/generator/cmd/protoc-gen-gclient -input-path=cmd/protoc-gen-gclient/testdata/rust/rust.bin
```

## General TODOs

- convert proto links into nice rustdoc
- fix documentation indentation after first line
3 changes: 1 addition & 2 deletions generator/src/genclient/genclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,7 @@ func (r *GenerateRequest) outDir() string {

// Output of generation.
type Output struct {
// TODO(codyoss): needs enough info to pass information back to protoc
// on what was generated.
// TODO(codyoss): https://github.com/googleapis/google-cloud-rust/issues/32
}

// Generate takes some state and applies it to a template to create a client
Expand Down
2 changes: 1 addition & 1 deletion generator/src/genclient/language/internal/golang/golang.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ func (c *Codec) HTTPPathArgs(h *genclient.HTTPInfo, state *genclient.APIState) [
var args []string
rawArgs := h.PathArgs()
for _, arg := range rawArgs {
// TODO(codyoss): handle nest path params
// TODO(codyoss): https://github.com/googleapis/google-cloud-rust/issues/34
args = append(args, ", req."+strcase.ToCamel(arg))
}
return args
Expand Down
8 changes: 0 additions & 8 deletions generator/src/genclient/language/internal/rust/rust.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,13 @@ import (
"github.com/iancoleman/strcase"
)

// TODO(codyoss): the current case converter is not working as intended for
// some fields: data_crc32c. Also most camel things in this file should be pascal
// case.

func NewCodec() *Codec {
return &Codec{}
}

type Codec struct{}

// TODO(codyoss): register reserved words and sanitize identifiers.

func (c *Codec) LoadWellKnownTypes(s *genclient.APIState) {
// TODO(codyoss): do better, maybe make our own types?
timestamp := &genclient.Message{
ID: ".google.protobuf.Timestamp",
Name: "String",
Expand Down Expand Up @@ -141,7 +134,6 @@ func (c *Codec) HTTPPathArgs(h *genclient.HTTPInfo, state *genclient.APIState) [
var args []string
rawArgs := h.PathArgs()
for _, arg := range rawArgs {
// TODO(codyoss): handle nest path params
args = append(args, "req."+strcase.ToSnake(arg))
}
return args
Expand Down
2 changes: 1 addition & 1 deletion generator/src/genclient/templatedata.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ func (m *message) Name() string {
}

func (m *message) DocLines() []string {
// TODO(codyoss): move this into codec to avoid strange whitespace things.
// TODO(codyoss): https://github.com/googleapis/google-cloud-rust/issues/33
ss := strings.Split(m.s.Documentation, "\n")
for i := range ss {
ss[i] = strings.TrimSpace(ss[i])
Expand Down
6 changes: 2 additions & 4 deletions generator/src/genclient/translator/protobuf/protobuf.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,10 @@ func NewTranslator(opts *Options) *Translator {
// Translates proto representation into a [genclient.GenerateRequest].
func (t *Translator) Translate() (*genclient.GenerateRequest, error) {
api := &genclient.API{
//TODO(codyoss): Read this from somewhere, likely service yaml or proto opt
//TODO(codyoss): https://github.com/googleapis/google-cloud-rust/issues/38
Name: "secretmanager",
}
files := t.request.GetSourceFileDescriptors()
// TODO(codyoss): maybe need to process well known types: https://github.com/googleapis/gapic-generator-go/blob/main/internal/gengapic/well_known_types.go
for _, f := range files {
var fileServices []*genclient.Service
fFQN := "." + f.GetPackage()
Expand Down Expand Up @@ -211,8 +210,7 @@ func (t *Translator) processMessage(m *descriptorpb.DescriptorProto, mFQN string
e := t.processEnum(e, mFQN, message)
message.Enums = append(message.Enums, e)
}
// TODO(codyoss): how to support one-ofs? For now ignore the issue
// and flatten.
// TODO(codyoss): https://github.com/googleapis/google-cloud-rust/issues/39
for _, mf := range m.Field {
field := &genclient.Field{}
field.Name = mf.GetName()
Expand Down
1 change: 0 additions & 1 deletion generator/templates/go/client.go.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ func doRequest(client *http.Client, req *http.Request) ([]byte, error){
if err != nil {
return nil, err
}
{{! TODO transform bytes into error type if error }}
return b, nil
}
{{#Messages}}
Expand Down
2 changes: 1 addition & 1 deletion generator/templates/rust/model.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ pub struct {{Name}} {
{{/Fields}}
}
{{#NestedMessages}}
{{! TODO(codyoss) We should consider handling these better }}
{{! TODO(codyoss) https://github.com/googleapis/google-cloud-rust/issues/40 }}
{{> model}}
{{/NestedMessages}}
{{#Enums}}
Expand Down

0 comments on commit f0da672

Please sign in to comment.