Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1447 +/- ##
==========================================
+ Coverage 85.15% 85.18% +0.02%
==========================================
Files 102 102
Lines 12984 13209 +225
==========================================
+ Hits 11057 11252 +195
- Misses 1440 1467 +27
- Partials 487 490 +3
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
aphralG
left a comment
There was a problem hiding this comment.
Could we add more logging around the file actions and the manifest file ?
| } | ||
|
|
||
| continue | ||
| case model.Add, model.Update: |
There was a problem hiding this comment.
Can we add a log for updating and adding for consistency ?
There was a problem hiding this comment.
The RenameFile function is already logging that
There was a problem hiding this comment.
Ok, just not sure renaming makes it clear that the file is being updated or added the way that its clear a file is being deleted. I wouldn't be able to tell immediately the action being preformed
| serverAddr := serverAddress(ctx, commandConfig) | ||
|
|
||
| slog.InfoContext(ctx, "Dialing grpc server", "server_addr", serverAddr) | ||
| slog.InfoContext(ctx, "Creating connection to management plane server", "server_address", serverAddr) |
There was a problem hiding this comment.
Could the type also be logged, auxiliary or command ?
There was a problem hiding this comment.
Yeah I'll add the server type to the context so that it gets logged here
Proposed changes
Checklist
Before creating a PR, run through this checklist and mark each as complete.
CONTRIBUTINGdocumentmake install-toolsand have attached any dependency changes to this pull requestREADME.md)