Skip to content
This repository has been archived by the owner on Jan 8, 2022. It is now read-only.

Fix colorscale #21

Open
wants to merge 12 commits into
base: stripe/master
Choose a base branch
from
Open

Fix colorscale #21

wants to merge 12 commits into from

Conversation

ChimeraCoder
Copy link

@ChimeraCoder ChimeraCoder commented Jun 25, 2019

Pull in Yelp#36

r? @sjung-stripe
cc @choo-stripe @stripe/observability

Rahul Ravindran and others added 12 commits June 25, 2019 17:58
This makes it possible to use red, light and dark green shades for
heatmaps and other charts.
Every time you run make test the deps task runs as it is a dependency.
If we stop this being a phony task, the deps task will only run if a
deps file exists. Touching this file after installing deps should ensure
this. This means its slightly faster to run make test. Running make
clean removes this file and will redo the deps process, at any point you
can remove the file and force the deps process to continue
This is useful to target specific tests and to allow passing the -i
option to make test which installs the test dependencies and makes
things run faster
goimports (https://godoc.org/golang.org/x/tools/cmd/goimports) formats
it like this and I have this set as a pre save command in my editor
This test sometimes passes, but mostly fails. The reason is the
iteration through the ChartColors map to find the palleteIndex - which
is explained nicely in
https://nathanleclaire.com/blog/2014/04/27/a-surprising-feature-of-golang-that-colored-me-impressed/

I did try creating my own *schema.ResourceData struct, but doing so
seemed to require a lot of knowledge of terraform internals and couple
the test to terraform internals. It seemed better and easier to test to
decouple the terraform internals from our logic and split out into two
functions - there may be a more idiomatic way of doing this.
To do this we change the datastructure from a map to a slice of structs,
as the ordering of slices will be consistent. This does mean we need
to loop through the slice to find the correct value where previously
we used a map lookup, but the map is small so the overhead shouldnt
be much
This matches the colors used at
https://developers.signalfx.com/reference#section-color-palette

I've had to make up names of the colors - where the existing names made
sense and didnt clash with another color i've used them. I also used
https://www.colorhexa.com to find an opinion on what the color was.
Copy link

@sjung-stripe sjung-stripe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code looks fine to me. be sure to build the provider and try a plan before merging

@CLAassistant
Copy link

CLAassistant commented Aug 6, 2020

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 4 committers have signed the CLA.

✅ ChimeraCoder
❌ drmorr0
❌ timmow
❌ rahulravindran0108
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants