Conversation
| @@ -0,0 +1,23 @@ | |||
| version: "3.9" | |||
There was a problem hiding this comment.
A docker compose file can be very handy when your project uses localstack, postgres or any other dependencies.
| @@ -0,0 +1,11 @@ | |||
| .PHONY: run | |||
There was a problem hiding this comment.
A makefile can help up local development, it's like adding command aliases.
|
|
||
| RUN apk add --no-cache git | ||
|
|
||
| # Set the Current Working Directory inside the container |
There was a problem hiding this comment.
In general, I don't like straightforward comments, they don't have much added value and just spam the code. In this instance, the WORKDIR command is very clear and doesn't need any more explanation. Although, I accept these when they appear in a tutorial type code to help someone new to the language/code.
|
|
||
| # Run the binary program produced by `go install` | ||
| ENTRYPOINT ["./deveui-cli"] No newline at end of file | ||
| FROM scratch AS production |
There was a problem hiding this comment.
Get familiar with multi stage builds, they can save you a lot of build time. This stage especially also saves a lot of space too. The original image size was 334MB (golang:1.20-alpine + code + binary), whereas now it's only 2.67MB (necessary folders + just the binary). You can check it by running docker images command.
| "github.com/kelseyhightower/envconfig" | ||
| ) | ||
|
|
||
| /* |
There was a problem hiding this comment.
I felt this comment was too much in a main file. Now there is a makefile that is self-explanatory, but if that is not enough then better to put any additional info in the readme.
| This is to handle any unexpected terminations of the program and to resume processing DevEUIs. | ||
| */ | ||
| func main() { | ||
| if err := godotenv.Load(".env"); err != nil { |
There was a problem hiding this comment.
I have a couple of issues with this. Having a .env file is fairly common, however if the file exists but a value is missing then this won't detect it. For example, if BASE_URL is missing, then os.Getenv will return with an empty string. If you wanted to handle that case then you'd need to do a check and set a default value. Another issue is the type conversions below, which is cumbersome and a lot of code. On the other hand, envconfig is very flexible, you can tell which env vars are required or optional, you can set default values and type conversion is dealt with too, See how much readable the code becomes with it.
| path, err := url.JoinPath(baseURL, endpoint) | ||
| if err != nil { | ||
| return nil, err | ||
| return nil, fmt.Errorf("failed to join url: %w", err) |
There was a problem hiding this comment.
Wrapping errors can help to identify where an error came from, especially when multiple functions can return the same error message. The https://github.com/pkg/errors package can give you stack trace along with the error, and their .Wrap method is convenient to use, but I try not to use this package for new projects anymore as per the description in the repo roadmap.
| return device, nil | ||
| } else { | ||
| return nil, errors.New(resp.Status) | ||
| if resp.StatusCode < 200 || resp.StatusCode > 299 { |
There was a problem hiding this comment.
Just to make it a bit more robust.
| } | ||
|
|
||
| // RegisterDevice registers a new device using LoraWAN external service | ||
| func (l *LoraWAN) RegisterDevice(ctx context.Context, newDevice *device.Device) error { |
There was a problem hiding this comment.
Having the device as a parameter makes this better, because
- the function previously did two things: 1. create a new device 2. register it. Now it has a single responsibility.
- now that we reduced coupling, testing is easier as well
1 participant
No description provided.