-
Notifications
You must be signed in to change notification settings - Fork 37
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
Remove Volume.size_in_mb
from api; improve pkg/queue
tests
#718
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportPatch coverage has no change and project coverage change:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## main #718 +/- ##
==========================================
- Coverage 58.33% 58.25% -0.09%
==========================================
Files 56 56
Lines 2705 2702 -3
==========================================
- Hits 1578 1574 -4
- Misses 982 983 +1
Partials 145 145
☔ View full report in Codecov by Sentry. |
Remove `Volume.size_in_mb` since its unused and its non trivial to actually use it for its intended purpose. Signed-off-by: Sanskar Jaiswal <[email protected]>
Before this commit, running `go vet ./...` would return: ``` pkg/queue/queue_test.go:50:6: call to (*T).Fatal from a non-test goroutine pkg/queue/queue_test.go:94:6: call to (*T).Fatal from a non-test goroutine ``` Signed-off-by: Sanskar Jaiswal <[email protected]>
@@ -142,8 +142,6 @@ message Volume { | |||
VolumeSource source = 4; | |||
// PartitionID is the uuid of the boot partition. | |||
optional string partition_id = 5; | |||
// Size is the size to resize this volume to. |
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.
I know it's an optional field, but shouldn't we be deprecating this first?
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.
cc: @richardcase
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.
We'd need to add reserved 6
and probably reserved "size_in_mb"
to the definition of Volume.
This PR is stale because it has been open 30 days with no activity. |
kind/bug
What this PR does / why we need it:
Remove
Volume.size_in_mb
since its unused and its non trivial to actually use it for its intended purpose.Furthermore, remove
t.Fatal
calls from non test goroutines. Currently runninggo vet ./...
would return:Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #553
Special notes for your reviewer:
Checklist: