🎨 Add rainbow colouring of steps label

[chmouel]

We were showing the same blue color for every steps, let's now cycle of a list of
colors.

• Includes tests (if functionality changed/added)
• Includes docs (if user facing)
• Commit messages follow commit message best practices

Demo: https://asciinema.org/a/hbJk1PqMFE8hleRpDsraPw0WB

Closes #285

Release Notes

Steps are now labelled with different colours instead of the same one 🎨

[tekton-robot]

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/cmd/taskrun/log_writer.go	93.3%	93.8%	0.4
pkg/formatted/color.go	Do not exist	63.6%

[gavinfish]

I love this😄

[tekton-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chmouel, gavinfish

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [chmouel]

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

[vdemeester]

Nice 👍🏼 one comment though 😅

[vdemeester]

There might be a race here if this is called in different goroutine 🤔

[chmouel]

yeah i tough about that but since we werent using it i didn't follow tough, what do you suggest? external lib for locking etc...

[chmouel]

and fyi i did test it with -race and it was running well but i'll just sync/lock/release it in case off,

[vdemeester]

@chmouel right, we might not be in the case of race condition in the cli, but if somebody uses this might hit it

[tekton-robot]

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/cmd/taskrun/log_writer.go	93.3%	93.8%	0.4
pkg/formatted/color.go	Do not exist	69.2%

[sthaha]

nit: usually this is done using rc.current = (rc.current +1) % len(rc.all) without having the need for an if

[sthaha]

mux must be part of the rc and not a package level variable; this allows for mulitple RainbowColors to be used without having one instance to wait for another instance.

[sthaha]

it is better to use Rlock and RWlocks

[sthaha]

mux should be instance variable and not package var since it only locks the rc.cached and not a package level variable.

[sthaha]

❤️ Neat!

some comments about sync.Mutex that needs addressing.

[tekton-robot]

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/cmd/taskrun/log_writer.go	93.3%	93.8%	0.4
pkg/formatted/color.go	Do not exist	78.9%

[vdemeester]

/lgtm
/meow boxes

[tekton-robot]

@vdemeester:

Details

In response to this:

/lgtm
/meow boxes

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[vdemeester]

/hold
To let @sthaha review it 👼

[chmouel]

red is avoided since we keep it for error messages, (probably should have put this in the comment instead of here)

[sthaha]

when I mentione RLock and Lock() I was hinting to having a single mutex in the rainbow. which will be RLocked for checking if the item is in cache and if not, Lock() to add the item in cache. but i like your implementation to create a round robin counter but for the counter itself, Get and increase isn't quite as useful as just a next which returns the next value.

please consider one of these implemention for counter. 1. that uses atomic, the other that uses a mutex

type atomicCounter struct {
  value     uint32
  threshold int
}

func (c *atomicCounter) next() int {
  v := atomic.AddUint32(&c.value, 1)
  next := int(v-1) % c.threshold
  atomic.CompareAndSwapUint32(&c.value, uint32(c.threshold), 0)
  return next

}

type counter struct {
  value     int
  threshold int
  m         sync.Mutex
}

func (c *counter) next() int {
  c.m.Lock()
  defer c.m.Unlock()

  v := c.value
  c.value = (v + 1) % c.threshold
  return v

}

and the rainbow would be some what like this

type rainbow struct {
  cache   sync.Map
  counter counter
}

func newRainbow() *rainbow {
  return &rainbow{
    counter: counter{threshold: len(palette)},
  }

}

func (r *rainbow) colorFor(x string) color {
  if value, ok := r.cache.Load(x); ok {
    return value.(color)
  }

  clr := palette[r.counter.next()]
  r.cache.Store(x, clr)
  return clr
}

[tekton-robot]

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/color.go	Do not exist	58.8%

[sthaha]

looks good to me expect for the Fprintf

[sthaha]

/lgtm