- 
                Notifications
    
You must be signed in to change notification settings  - Fork 11
 
Setup buf modules and cleanup README #3
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
| 
           The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request). 
  | 
    
        
          
                README.md
              
                Outdated
          
        
      | ) | ||
| 
               | 
          ||
| // Weather data byte slice. | ||
| var weatherDataBytes = []byte{0xa, 0x7, 0x53, 0x65, 0x61, 0x74, 0x74, 0x6c, 0x65, 0x12, 0x1d, 0xa, 0x5, 0x4b, 0x41, 0x44, 0x39, 0x33, 0x15, 0x66, 0x86, 0x22, 0x43, 0x1d, 0xcd, 0xcc, 0x34, 0x41, 0x25, 0xd7, 0xa3, 0xf0, 0x41, 0x2d, 0x33, 0x33, 0x13, 0x40, 0x30, 0x3, 0x12, 0x1d, 0xa, 0x5, 0x4b, 0x48, 0x42, 0x36, 0x30, 0x15, 0xcd, 0x8c, 0x22, 0x43, 0x1d, 0x33, 0x33, 0x5b, 0x41, 0x25, 0x52, 0xb8, 0xe0, 0x41, 0x2d, 0x33, 0x33, 0xf3, 0x3f, 0x30, 0x3} | 
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.
Can you format this so it doesn't wrap the screen?
        
          
                README.md
              
                Outdated
          
        
      | "google.golang.org/protobuf/proto" | ||
| 
               | 
          ||
| weatherv1 "github.com/..." | ||
| weatherv1 "buf.build/go/hyperpb/internal/gen/example/weather/v1" | 
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.
Can we use the expected generated SDK URL here?
        
          
                README.md
              
                Outdated
          
        
      | weatherv1 "buf.build/go/hyperpb/internal/gen/example/weather/v1" | ||
| ) | ||
| 
               | 
          ||
| // Weather data byte slice. | 
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.
Can you convert this to JSON and include that in the comment?
| our binary, and parse some data with it. | ||
| 
               | 
          ||
| ```go | ||
| package main | 
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.
Can you make sure these are all synced up w/ example_test.go? Might also be good to include comments on both sides to say that they need to be in sync.
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 good, some details.
This is a follow-up to #3 to use generated SDKs for examples.
This PR does some simple clean-ups:
and adjusts the
buf.gen.*yamlfiles accordingly. It alsoconfigures the
inputskey in thebuf.gen.*yamlfiles, sothey don't need to be managed in the
Makefile.weather module to the BSR.
properly when pushed to the BSR. However, since the repo is
currently private, the logo 404's. This should be fixed when
the repo is public.
and the subsequent examples are clear function definitions.
A follow-up PR will be made to use generated SDKs for examples.
We skip
buf breakingfor this PR inbuf-actionsince this PR breaksthe workspace into new modules.