Generate steps name when none has been provided #611

chmouel · 2020-01-15T18:56:26Z

Let's generate the step name if the user has not provided one, like what is done
pipeline (which would shows up in tr)

Before:

After:

Closes #[593]

Changes

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

Includes tests (if functionality changed/added)
Includes docs (if user facing)
Regenerate the manpages and docs with make docs and make man if needed.
Run the code checkers with make check
Commit messages follow commit message best practices

See the contribution guide
for more details.

Release Notes

release-note

tekton-robot · 2020-01-15T19:09:52Z

The following is the coverage report on pkg/.
Say /test pull-tekton-cli-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/formatted/k8s.go	6.2%	5.0%	-1.2

tekton-robot · 2020-01-15T19:17:46Z

The following is the coverage report on pkg/.
Say /test pull-tekton-cli-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/formatted/k8s.go	6.2%	5.0%	-1.2

tekton-robot · 2020-01-15T19:37:46Z

The following is the coverage report on pkg/.
Say /test pull-tekton-cli-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/formatted/k8s.go	6.2%	5.0%	-1.2

pkg/formatted/k8s.go

pkg/cmd/task/describe.go

sthaha

lgtm
it may be better to start counting from 1 instead of 0

vdemeester

As far as I remember, the index in the name of the step is the index of it in the array and it starts at 0 because of go 👼 (so an unamed step at the 3rd step, would be named step-unamed-2)

tekton-robot · 2020-01-16T10:20:28Z

The following is the coverage report on pkg/.
Say /test pull-tekton-cli-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/formatted/k8s.go	6.2%	4.8%	-1.5

chmouel · 2020-01-16T10:26:30Z

pkg/formatted/k8s.go

+// AutoStepName when our stepName is empty return a genrated name as generated
+// on pipeLine
+func AutoStepName(stepName string) string {
+	if stepName != "" {


I do this because I could not find a way to do this nicely in builtin gotemplate....

Just making sure .. so we want to print something like ..

- A named step - unnamed-2 - another-named-step - unnamed-4

right?

Just saw that the test confirms it and it makes sense!

tekton-robot · 2020-01-16T10:26:35Z

The following is the coverage report on pkg/.
Say /test pull-tekton-cli-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/formatted/k8s.go	6.2%	4.8%	-1.5

vdemeester

/hold
We should have a test with an unamed step in the middle of two named step 👼

chmouel · 2020-01-16T20:15:07Z

Sure! done...

/hold cancel

tekton-robot · 2020-01-16T20:26:13Z

The following is the coverage report on pkg/.
Say /test pull-tekton-cli-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/formatted/k8s.go	6.2%	4.8%	-1.5

tekton-robot · 2020-01-16T20:46:43Z

The following is the coverage report on pkg/.
Say /test pull-tekton-cli-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/formatted/k8s.go	6.2%	28.6%	22.3

pkg/cmd/task/describe_test.go

tekton-robot · 2020-01-17T09:16:46Z

The following is the coverage report on pkg/.
Say /test pull-tekton-cli-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/formatted/k8s.go	6.2%	31.8%	25.6

tekton-robot · 2020-01-17T09:20:31Z

The following is the coverage report on pkg/.
Say /test pull-tekton-cli-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/formatted/k8s.go	6.2%	31.8%	25.6

tekton-robot · 2020-01-17T10:11:36Z

The following is the coverage report on pkg/.
Say /test pull-tekton-cli-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/formatted/k8s.go	6.2%	31.8%	25.6

vdemeester · 2020-01-17T15:22:24Z

pkg/formatted/k8s.go

+		return stepName
+	}
+	unnamedStep := fmt.Sprintf("unnamed-%d", stepCounter)
+	atomic.AddUint64(&stepCounter, 1)


nit:

unnamedStep := fmt.Sprintf("unnamed-%d", stepCounter) atomic.AddUint64(&stepCounter, 1) if stepName != "" { return stepName } return unnamedStep

tekton-robot · 2020-01-17T15:22:34Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danielhelfand, vdemeester

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:

~~OWNERS~~ [danielhelfand,vdemeester]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Closes tektoncd#593 Signed-off-by: Chmouel Boudjnah <[email protected]>

tekton-robot · 2020-01-19T21:36:41Z

The following is the coverage report on pkg/.
Say /test pull-tekton-cli-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/formatted/k8s.go	6.2%	28.6%	22.3

sthaha

/lgtm

tekton-robot requested review from hrishin and sthaha January 15, 2020 18:56

googlebot added the cla: yes label Jan 15, 2020

tekton-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 15, 2020

chmouel force-pushed the issue-593-tkn-assumes-steps-that-has-name branch from 0a58595 to 86a614b Compare January 15, 2020 19:04

chmouel force-pushed the issue-593-tkn-assumes-steps-that-has-name branch from 86a614b to 9fef44b Compare January 15, 2020 19:24

sthaha reviewed Jan 16, 2020

View reviewed changes

pkg/formatted/k8s.go Outdated Show resolved Hide resolved

sthaha reviewed Jan 16, 2020

View reviewed changes

pkg/formatted/k8s.go Outdated Show resolved Hide resolved

sthaha reviewed Jan 16, 2020

View reviewed changes

pkg/cmd/task/describe.go Outdated Show resolved Hide resolved

sthaha reviewed Jan 16, 2020

View reviewed changes

pkg/cmd/task/describe.go Outdated Show resolved Hide resolved

sthaha reviewed Jan 16, 2020

View reviewed changes

vdemeester requested changes Jan 16, 2020

View reviewed changes

chmouel changed the title ~~generate steps name when none has been provided~~ Generate steps name when none has been provided Jan 16, 2020

chmouel force-pushed the issue-593-tkn-assumes-steps-that-has-name branch from 9fef44b to 7170505 Compare January 16, 2020 10:07

chmouel force-pushed the issue-593-tkn-assumes-steps-that-has-name branch from 7170505 to 1e35846 Compare January 16, 2020 10:21

chmouel commented Jan 16, 2020

View reviewed changes

danielhelfand approved these changes Jan 16, 2020

View reviewed changes

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 16, 2020

vdemeester reviewed Jan 16, 2020

View reviewed changes

tekton-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 16, 2020

chmouel force-pushed the issue-593-tkn-assumes-steps-that-has-name branch from 1e35846 to 51ff027 Compare January 16, 2020 20:13

tekton-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 16, 2020

chmouel force-pushed the issue-593-tkn-assumes-steps-that-has-name branch from 51ff027 to f56567e Compare January 16, 2020 20:33

tekton-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 16, 2020

vdemeester requested changes Jan 17, 2020

View reviewed changes

pkg/cmd/task/describe_test.go Outdated Show resolved Hide resolved

pkg/cmd/task/describe_test.go Outdated Show resolved Hide resolved

chmouel force-pushed the issue-593-tkn-assumes-steps-that-has-name branch 2 times, most recently from 676d2ae to 95a01e3 Compare January 17, 2020 09:16

chmouel force-pushed the issue-593-tkn-assumes-steps-that-has-name branch from 95a01e3 to 4f2da22 Compare January 17, 2020 09:58

tekton-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 17, 2020

vdemeester approved these changes Jan 17, 2020

View reviewed changes

danielhelfand mentioned this pull request Jan 17, 2020

Add tkn clustertask describe Command #624

Merged

5 tasks

generate steps name when none has been provided

eb2805f

Closes tektoncd#593 Signed-off-by: Chmouel Boudjnah <[email protected]>

chmouel force-pushed the issue-593-tkn-assumes-steps-that-has-name branch from 4f2da22 to eb2805f Compare January 19, 2020 21:32

sthaha reviewed Jan 20, 2020

View reviewed changes

tekton-robot assigned sthaha Jan 20, 2020

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 20, 2020

tekton-robot merged commit e432960 into tektoncd:master Jan 20, 2020

chmouel deleted the issue-593-tkn-assumes-steps-that-has-name branch January 20, 2020 07:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generate steps name when none has been provided #611

Generate steps name when none has been provided #611

chmouel commented Jan 15, 2020 •

edited

Loading

tekton-robot commented Jan 15, 2020

tekton-robot commented Jan 15, 2020

tekton-robot commented Jan 15, 2020

sthaha left a comment

vdemeester left a comment

tekton-robot commented Jan 16, 2020

chmouel Jan 16, 2020

sthaha Jan 20, 2020

sthaha Jan 20, 2020

tekton-robot commented Jan 16, 2020

vdemeester left a comment

chmouel commented Jan 16, 2020

tekton-robot commented Jan 16, 2020

tekton-robot commented Jan 16, 2020

tekton-robot commented Jan 17, 2020

tekton-robot commented Jan 17, 2020

tekton-robot commented Jan 17, 2020

vdemeester Jan 17, 2020

tekton-robot commented Jan 17, 2020

tekton-robot commented Jan 19, 2020

sthaha left a comment

Generate steps name when none has been provided #611

Generate steps name when none has been provided #611

Conversation

chmouel commented Jan 15, 2020 • edited Loading

Changes

Submitter Checklist

Release Notes

tekton-robot commented Jan 15, 2020

tekton-robot commented Jan 15, 2020

tekton-robot commented Jan 15, 2020

sthaha left a comment

Choose a reason for hiding this comment

vdemeester left a comment

Choose a reason for hiding this comment

tekton-robot commented Jan 16, 2020

chmouel Jan 16, 2020

Choose a reason for hiding this comment

sthaha Jan 20, 2020

Choose a reason for hiding this comment

sthaha Jan 20, 2020

Choose a reason for hiding this comment

tekton-robot commented Jan 16, 2020

vdemeester left a comment

Choose a reason for hiding this comment

chmouel commented Jan 16, 2020

tekton-robot commented Jan 16, 2020

tekton-robot commented Jan 16, 2020

tekton-robot commented Jan 17, 2020

tekton-robot commented Jan 17, 2020

tekton-robot commented Jan 17, 2020

vdemeester Jan 17, 2020

Choose a reason for hiding this comment

tekton-robot commented Jan 17, 2020

tekton-robot commented Jan 19, 2020

sthaha left a comment

Choose a reason for hiding this comment

chmouel commented Jan 15, 2020 •

edited

Loading