add helper for inferring index name from input #506
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
size/M
ahmetb
force-pushed
the
plugin-name-infer
branch
from
d26446b to
30ce1b8
Compare
To be used in multi-index work. Signed-off-by: Ahmet Alp Balkan <[email protected]>
ahmetb
force-pushed
the
plugin-name-infer
branch
from
30ce1b8 to
ca81ca7
Compare
Codecov Report
@@ Coverage Diff @@
## master #506 +/- ##
==========================================
+ Coverage 58.51% 58.72% +0.21%
==========================================
Files 22 22
Lines 969 974 +5
==========================================
+ Hits 567 572 +5
Misses 346 346
Partials 56 56
Continue to review full report at Codecov.
|
|
I can use |
|
Migration codes ideally should not use external variables like this (we want them to do a rather static job). |
| {"a/b/c", "a", "b/c"}, // despite unsupported | ||
| } | ||
| for _, tt := range tests { | ||
| t.Run(tt.in, func(t *testing.T) { |
There was a problem hiding this comment.
We are not entirely consistent about how test cases and the inner *testing.T is called. In some places, the inner is tt *testing.T in others just t.
I personally would prefer to have consistency here an only use tt for the inner *testing.T. However, we can do that on the whole repo in a follow-up PR.
There was a problem hiding this comment.
Opened #508 to clarify this.
There was a problem hiding this comment.
This is mostly because we've used different IDEs over time (vscode-go and Goland).
Similarly you might notice I added a = after the ...%q, want = here. Most other unit-tests don't have the =. :)
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ahmetb, corneliusweig The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
To be used in multi-index work.
cc: @chriskim06