webui icon indicating copy to clipboard operation
webui copied to clipboard

Fix context UI add formatting refactor

Open chanwit opened this issue 4 years ago • 4 comments

This PR

  • fixed K8s context listing bug
  • added code formatting for TypeScript and Go, including ESLit and Go Vet
  • did some refactoring
  • updated CRDs to those of Flux v0.17.0

chanwit avatar Sep 09 '21 09:09 chanwit

Thanks @chanwit! Would you please split this into PRs that accomplish one thing (or near enough) -- maybe

  • put formatting and linting in the build
  • fix bug and refactoring (if the refactoring pertains to the code you were fixing)
  • update the CRDs (unless that is part of fixing, or refactoring)

squaremo avatar Sep 09 '21 09:09 squaremo

@jpellizzari I am in process of splitting this PR into smaller ones as @squaremo pointed out.

For the bug fixing part, there's a bug when we obtaining the list of contexts. Please see its structure and you'll see that we used the wrong field, c.Cluster. c.Cluster is the cluster name, but not the context name. The context name appears to be the key portion of rules.Contexts map. I found this bug accidentally because a cluster name and a context on my machine are different.

chanwit avatar Sep 09 '21 14:09 chanwit

@jpellizzari I am in process of splitting this PR into smaller ones as @squaremo pointed out.

For the bug fixing part, there's a bug when we obtaining the list of contexts. Please see its structure and you'll see that we used the wrong field, c.Cluster. c.Cluster is the cluster name, but not the context name. The context name appears to be the key portion of rules.Contexts map. I found this bug accidentally because a cluster name and a context on my machine are different.

Excellent! Nice find. I will await those PRs; feel free to add me as a reviewer when they are ready. Thanks for the contribution.

jpellizzari avatar Sep 09 '21 14:09 jpellizzari

we used the wrong field, c.Cluster. c.Cluster is the cluster name, but not the context name. The context name appears to be the key portion of rules.Contexts map.

Please be sure to put this explanation in the commit message :-)

squaremo avatar Sep 09 '21 14:09 squaremo