naming: consolidate task naming into single package#1694
naming: consolidate task naming into single package#1694aluzzardi merged 2 commits intomoby:masterfrom
Conversation
manager/orchestrator/task.go
Outdated
| // Assign name based on task name schema | ||
| name := store.TaskName(&task) | ||
| name := naming.Task(&task) | ||
| task.Annotations = api.Annotations{Name: name} |
There was a problem hiding this comment.
Should we remove this line that stores the task name in raft, as discussed previously?
cc @aluzzardi
There was a problem hiding this comment.
As part of this PR?
There was a problem hiding this comment.
You can split it into a separate branch if it's cleaner, but it's literally here ^^^
Since you're stopping by it would be too bad not to change it
There was a problem hiding this comment.
Added here as a separate commit. We basically get rid of this whole clause.
|
Thanks for doing this! |
|
This is missing naming.go :) +1 on @aaronlehmann's comment - we're a line away from removing naming from raft, I think it should be done in this PR as well Overall LGTM |
|
|
This change consolidates task naming into a single package. The names of tasks was at risk of diverging over time, since it is a calculated property. We can further extend this package to do more naming and validation work. Signed-off-by: Stephen J Day <[email protected]>
stevvooe
force-pushed
the
naming-package
branch
from
5a322b8 to
57c74d6
Compare
Current coverage is 55.94% (diff: 83.33%)@@ master #1694 diff @@
==========================================
Files 89 90 +1
Lines 14510 14507 -3
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
- Hits 8143 8116 -27
- Misses 5278 5303 +25
+ Partials 1089 1088 -1
|
Signed-off-by: Stephen J Day <[email protected]>
|
LGTM |
|
Thanks! LGTM |
5 participants
This change consolidates task naming into a single package. The names of
tasks was at risk of diverging over time, since it is a calculated
property. We can further extend this package to do more naming and
validation work.
Signed-off-by: Stephen J Day [email protected]