-
Notifications
You must be signed in to change notification settings - Fork 17
Fix context UI add formatting refactor #57
base: main
Are you sure you want to change the base?
Changes from all commits
12d02f2
3864290
b5d5f62
e1bec12
858defe
2a03952
c19baec
e28a229
590ea3c
de62f48
e8301cb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,3 +2,4 @@ node_modules | |
.cache | ||
dist | ||
bin | ||
.idea/* |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,22 @@ | ||
.PHONY: clean all test assets dev proto download-crd-deps | ||
SOURCE_VERSION := v0.13.2 | ||
KUSTOMIZE_VERSION := v0.12.2 | ||
HELM_CRD_VERSION := v0.10.1 | ||
SOURCE_CONTROLLER_VERSION := v0.15.4 | ||
KUSTOMIZE_CONTROLLER_VERSION := v0.14.0 | ||
HELM_CONTROLLER_VERSION := v0.11.2 | ||
|
||
all: test build | ||
all: dist/index.html fmt vet test build | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We probably shouldn't do "write" operations (formatting) on the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We did that in Flux CLI. Go Fmt is part of the make file. Should we do this @stefanprodan? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with Jordan here; I think it's OK to have phony targets that rewrite files, but the main build shouldn't create (more) diffs. If it does, it encourages commits which contain code changes then some random reformatting elsewhere. This can make it difficult to reconcile with builds in which you commit generated files. For that, you can make people explicitly regenerate the file, and have a guard in the tests (and CI) to make sure the committed, generated file is correct. (Also see There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (You can call me a hypocrite, since all the flux controllers inherit a Makefile structure which does |
||
|
||
dist/index.html: | ||
npm run build | ||
|
||
fmt: | ||
go fmt ./... | ||
npx prettier --write 'ui/**/*.ts(x)?' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think one formatter is the way to go here. Two formatters may end up conflicting with one another. |
||
npx eslint ui/ --ext .ts --ext .tsx --fix | ||
|
||
vet: | ||
go vet ./... | ||
npx eslint ui/ --ext .ts --ext .tsx | ||
|
||
test: | ||
go test ./... | ||
|
||
|
@@ -21,8 +30,12 @@ proto: pkg/rpc/clusters/clusters.proto | |
protoc pkg/rpc/clusters/clusters.proto --twirp_out=./ --go_out=. --twirp_typescript_out=./ui/lib/rpc | ||
|
||
download-crd-deps: | ||
curl -s https://raw.githubusercontent.com/fluxcd/source-controller/${SOURCE_VERSION}/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml > tools/crd/gitrepository.yaml | ||
curl -s https://raw.githubusercontent.com/fluxcd/source-controller/${SOURCE_VERSION}/config/crd/bases/source.toolkit.fluxcd.io_buckets.yaml > tools/crd/bucket.yaml | ||
curl -s https://raw.githubusercontent.com/fluxcd/kustomize-controller/${KUSTOMIZE_VERSION}/config/crd/bases/kustomize.toolkit.fluxcd.io_kustomizations.yaml > tools/crd/kustomization.yaml | ||
curl -s https://raw.githubusercontent.com/fluxcd/helm-controller/${HELM_CRD_VERSION}/config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml > tools/crd/helmrelease.yaml | ||
curl -s https://raw.githubusercontent.com/fluxcd/source-controller/${SOURCE_VERSION}/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml > tools/crd/helmrepository.yaml | ||
curl -s https://raw.githubusercontent.com/fluxcd/source-controller/${SOURCE_CONTROLLER_VERSION}/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml > tools/crd/gitrepository.yaml | ||
curl -s https://raw.githubusercontent.com/fluxcd/source-controller/${SOURCE_CONTROLLER_VERSION}/config/crd/bases/source.toolkit.fluxcd.io_buckets.yaml > tools/crd/bucket.yaml | ||
curl -s https://raw.githubusercontent.com/fluxcd/kustomize-controller/${KUSTOMIZE_CONTROLLER_VERSION}/config/crd/bases/kustomize.toolkit.fluxcd.io_kustomizations.yaml > tools/crd/kustomization.yaml | ||
curl -s https://raw.githubusercontent.com/fluxcd/helm-controller/${HELM_CONTROLLER_VERSION}/config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml > tools/crd/helmrelease.yaml | ||
curl -s https://raw.githubusercontent.com/fluxcd/source-controller/${SOURCE_CONTROLLER_VERSION}/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml > tools/crd/helmrepository.yaml | ||
|
||
clean: | ||
rm -rf dist/* || true | ||
rm ./bin/webui || true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may cause conflicts with
typescript
. Having two formmatters running will cause them to over-write each others' changes.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not very good at this kind of configuration.
Did you mean that we could just use
eslint --fix
and the source would get formatted without using prettier? If so, I'm happy to remove it.