Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: remove dead source code from UI folder #21446

Merged
merged 4 commits into from
May 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
32 changes: 6 additions & 26 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,30 +115,16 @@ Currently the project only depends on `git` and `bzr`.

You need a recent stable version of Rust. We recommend using [rustup](https://rustup.rs/) to install Rust.

You also need `clang`, `make`, `pkg-config`, `protobuf`, and `yarn` installed.
You also need `clang`, `make`, `pkg-config`, and `protobuf` installed.

- OSX: `brew install make pkg-config protobuf yarn`
- Linux (Arch): `pacman -S clang make pkgconf protobuf yarn`
- Linux (Ubuntu, RHEL): See below

#### Ubuntu-specific instructions:

For Ubuntu, you need to change the apt repository for `yarn`:

```
sudo apt remove yarn cmdtest
wget -qO- https://dl.yarnpkg.com/debian/pubkey.gpg | sudo apt-key add -
sudo apt-add-repository "deb https://dl.yarnpkg.com/debian/ stable main"

sudo apt install make clang pkg-config protobuf-compiler libprotobuf-dev yarn
```
- OSX: `brew install make pkg-config protobuf`
- Linux (Arch): `pacman -S clang make pkgconf protobuf`
- Linux (Ubuntu): `sudo apt install make clang pkg-config protobuf-compiler libprotobuf-dev`
- Linux (RHEL): See below

#### Redhat-specific instructions

For RedHat, there are some extra steps:

1. You must enable the [EPEL](https://fedoraproject.org/wiki/EPEL)
2. You must add the `yarn` [repository](https://yarnpkg.com/lang/en/docs/install/#centos-stable)
For RedHat, you must enable the [EPEL](https://fedoraproject.org/wiki/EPEL)

### Building with make

Expand Down Expand Up @@ -203,12 +189,6 @@ This project is built from various languages. To run test for all langauges and
$ make test
```

To run tests for just the Javascript component use:

```bash
$ make test-js
```

To run tests for just the Go/Rust components use:

```bash
Expand Down
70 changes: 10 additions & 60 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

# SUBDIRS are directories that have their own Makefile.
# It is required that all SUBDIRS have the `all` and `clean` targets.
SUBDIRS := http ui chronograf storage
SUBDIRS := http chronograf storage

export GOPATH=$(shell go env GOPATH)
export GOOS=$(shell go env GOOS)
Expand Down Expand Up @@ -69,18 +69,16 @@ SOURCES := $(shell find . -name '*.go' -not -name '*_test.go') go.mod go.sum
# All go source files excluding the vendored sources.
SOURCES_NO_VENDOR := $(shell find . -path ./vendor -prune -o -name "*.go" -not -name '*_test.go' -print)

# All assets for chronograf
UISOURCES := $(shell find ui -type f -not \( -path ui/build/\* -o -path ui/node_modules/\* -o -path ui/.cache/\* -o -name Makefile -prune \) )

# All precanned dashboards
PRECANNED := $(shell find chronograf/canned -name '*.json')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This wasn't used anywhere else that I could find, so I'm getting rid of it.


# List of binary cmds to build
CMDS := \
bin/$(GOOS)/influx \
bin/$(GOOS)/influxd

all: $(SUBDIRS) generate $(CMDS)
all: ui/build $(SUBDIRS) generate $(CMDS)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't want to have a Makefile in the UI folder, so I had to put the ui/build target here and in the generate command below. I don't love this but couldn't come up with any other way to make sure the built assets exist somewhere before the embedding commands happen. Right now all the embedding stuff is in the chronograf folder, and the embedding commands require the ui/build path to be where the built UI is. Definitely open to suggestions on how to do this more cleanly!

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's OK, we can cleanup in stages 👍


# Target for the built UI assets directory.
ui/build:
scripts/fetch-ui-assets.sh

# Target to build subdirs.
# Each subdirs must support the `all` target.
Expand All @@ -101,28 +99,6 @@ influxd: bin/$(GOOS)/influxd

influx: bin/$(GOOS)/influx

#
# Define targets for the web ui
#

node_modules: ui/node_modules

# phony target to wait for server to be alive
ping:
./etc/pinger.sh

e2e: ping
make -C ui e2e

chronograf_lint:
make -C ui lint

ui/node_modules:
make -C ui node_modules

ui_client:
make -C ui client

#
# Define action only targets
#
Expand All @@ -143,10 +119,7 @@ checktidy:
checkgenerate:
./etc/checkgenerate.sh

generate: $(SUBDIRS)

test-js: node_modules
make -C ui test
generate: ui/build $(SUBDIRS)

test-go:
$(GO_TEST) $(GO_TEST_PATHS)
Expand All @@ -167,7 +140,7 @@ test-integration: GO_TAGS=integration
test-integration:
$(GO_TEST) -count=1 $(GO_TEST_PATHS)

test: test-go test-js
test: test-go

test-go-race:
$(GO_TEST) -v -race -count=1 $(GO_TEST_PATHS)
Expand All @@ -187,26 +160,7 @@ clean:
@for d in $(SUBDIRS); do $(MAKE) -C $$d clean; done
$(RM) -r bin
$(RM) -r dist

define CHRONOGIRAFFE
._ o o
\_`-)|_
,"" _\_
," ## | 0 0.
," ## ,-\__ `.
," / `--._;) - "HAI, I'm Chronogiraffe. Let's be friends!"
," ## /
," ## /
endef
export CHRONOGIRAFFE
chronogiraffe: $(SUBDIRS) generate $(CMDS)
@echo "$$CHRONOGIRAFFE"

run: chronogiraffe
./bin/$(GOOS)/influxd --assets-path=ui/build

run-e2e: chronogiraffe
./bin/$(GOOS)/influxd --assets-path=ui/build --e2e-testing --store=memory
$(RM) -r ui/build

# generate feature flags
flags:
Expand All @@ -215,10 +169,6 @@ flags:
docker-image-influx:
@cp .gitignore .dockerignore
@docker image build -t influxdb:dev --target influx .

docker-image-ui:
@cp .gitignore .dockerignore
@docker image build -t influxui:dev --target ui .

dshell-image:
@cp .gitignore .dockerignore
Expand All @@ -228,4 +178,4 @@ dshell: dshell-image
@docker container run --rm -p 8086:8086 -p 8080:8080 -u $(shell id -u) -it -v $(shell pwd):/code -w /code influxdb:dshell

# .PHONY targets represent actions that do not create an actual file.
.PHONY: all $(SUBDIRS) run fmt checkfmt tidy checktidy checkgenerate test test-go test-js test-go-race test-tls bench clean node_modules vet nightly chronogiraffe dist ping protoc e2e run-e2e influxd libflux flags dshell dclean docker-image-flux docker-image-influx pkg-config
.PHONY: all $(SUBDIRS) run fmt checkfmt tidy checktidy checkgenerate test test-go test-go-race test-tls bench clean node_modules vet nightly dist protoc influxd libflux flags dshell dclean docker-image-flux docker-image-influx pkg-config
8 changes: 4 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ If you re-generate a file and find that `staticcheck` has failed, please see thi

#### End-to-End Tests

CI also runs end-to-end tests. These test the integration between the influx server the ui. You can run them locally in two steps:

- Start the server in "testing mode" by running `make run-e2e`.
- Run the tests with `make e2e`.
CI also runs end-to-end tests. These test the integration between the `influxd` server the UI.
Since the UI is used by interal repositories as well as the `influxdb` repository, the
end-to-end tests cannot be run on forked pull requests or run locally. The extent of end-to-end
testing required for forked pull requests will be determined as part of the review process.
6 changes: 3 additions & 3 deletions ui/fetch_ui_assets.sh → scripts/fetch-ui-assets.sh
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@
# respective releases in "influxdata/ui" (OSS-2.0, OSS-2.1, etc). Those releases
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved this file in the scripts directory. I didn't want to end up with anything in the ui directory other than the README.md.

# are updated only when a bug fix needs included for the UI of that OSS release.

curl -L https://github.com/influxdata/ui/releases/download/OSS-Master/build.tar.gz --output build.tar.gz
tar -xzf build.tar.gz
rm build.tar.gz
curl -L https://github.com/influxdata/ui/releases/download/OSS-Master/build.tar.gz --output ui/build.tar.gz
tar -xzf ui/build.tar.gz -C ui
rm ui/build.tar.gz
3 changes: 0 additions & 3 deletions ui/.browserslistrc

This file was deleted.

1 change: 0 additions & 1 deletion ui/.dockerignore

This file was deleted.

2 changes: 0 additions & 2 deletions ui/.eslintignore

This file was deleted.

75 changes: 0 additions & 75 deletions ui/.eslintrc.js

This file was deleted.

1 change: 0 additions & 1 deletion ui/.npmrc

This file was deleted.

6 changes: 0 additions & 6 deletions ui/.prettierrc.json

This file was deleted.

33 changes: 0 additions & 33 deletions ui/Makefile

This file was deleted.

19 changes: 3 additions & 16 deletions ui/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@

UI assets for InfluxDB are automatically downloaded and embedded in the `influxd` binary
when using the top-level `Makefile`. The UI assets are built and made available from
the [`influxdata/ui` repository](https://github.com/influxdata/ui). Currently, this `ui`
folder and its contents are being kept to preserve the ability to run end-to-end tests via Cypress in this repository against the built UI when changes are made to `influxdb`.
the [`influxdata/ui` repository](https://github.com/influxdata/ui). All of the UI source code
has been removed from this directory, and now lives in the [`influxdata/ui` repository](https://github.com/influxdata/ui).
Please submit all PRs and issues related to the InfluxDB UI to the [`influxdata/ui` repository](https://github.com/influxdata/ui).

### Starting a Local Development Environment

Expand Down Expand Up @@ -39,17 +40,3 @@ if the `ui` folder containing built assets is at the same level as the `influxdb
and the `influxd` binary is at `influxdb/bin/darwin/influxd`:

`$ ./bin/darwin/influxd --assets-path=../ui/build`

### Cypress Testing

For the end to end tests to run properly, the server needs to be running in the e2e testing mode with the in-memory data store. From the `influxdb` directory:

`$ ./bin/darwin/influxd --e2e-testing --store=memory`

From the ui directory, install the packages necessary for testing:

`$ yarn install`

To run Cypress locally:

`$ yarn cy`
1 change: 0 additions & 1 deletion ui/__mocks__/@influxdata/flux-parser.ts

This file was deleted.

Loading