-
Notifications
You must be signed in to change notification settings - Fork 70
Conversation
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.
If tests pass,
pkg/lifecycle/daemon/daemon_test.go
Outdated
@@ -194,6 +196,7 @@ func TestDaemonAPI(t *testing.T) { | |||
|
|||
for _, test := range tests { | |||
t.Run(test.name, func(t *testing.T) { | |||
fmt.Println("name", test.name) |
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.
Kill this
@@ -26,6 +27,16 @@ func (d *NavcycleRoutes) getStep(c *gin.Context) { | |||
return | |||
} | |||
|
|||
if preExecuteFunc, exists := d.PreExecuteFuncMap[step.ShortName()]; exists { | |||
if err := preExecuteFunc(context.Background(), step); err != nil { |
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.
Should we be using background context here? What do these preExecuteFuncs do?
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.
OK, looks like your current use of this does not use context yet, but this is still worth considering before we merge.
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.
discussed irl - background context from the request handler is passed to pre execute funcs
What I Did
How I Did it
rootFs
to ensure proper location of overlaysHow to verify it
Description for the Changelog
Picture of a Boat (not required but encouraged)
🛶