Special-case render so it doesn't appear as a task #3000
Conversation
justinsb
force-pushed
the
dont_make_render_a_task
branch
from
5e4259b to
921fde8
Compare
This avoids problems when we are approving packages with tasks.
justinsb
force-pushed
the
dont_make_render_a_task
branch
from
921fde8 to
a7927bf
Compare
| // TODO: Safe to assume it's always a render? | ||
| message = "Internal commit (render)" | ||
| } |
There was a problem hiding this comment.
why not instead return more general type from the Apply function; it can either carry information to add a task, or internal message for non-task things.
btw, the non-task changes seem to break the model - what should constitute a task and what shouldn't?
There was a problem hiding this comment.
Good idea - I'll give that a go.
If we're always going to run render, I don't know that we should show it. (What happens if a user deletes it - do we just add it back?) I think in general we should show everything, but specifically here for a task we automatically add it makes the synchronization between client state and server state more complicated - the value I get on the server after an apply has an extra task. There's plenty of precedent for that in kubernetes (e.g. sidecar injection), but it does make the objects harder to work with (e.g. I think it breaks kubectl diff)
2 participants
This avoids problems when we are approving packages with tasks.