-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Conversation
87260ac
to
cc4b0d5
Compare
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') |
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 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) |
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 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!
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 think it's OK, we can cleanup in stages 👍
@@ -12,6 +12,6 @@ | |||
# respective releases in "influxdata/ui" (OSS-2.0, OSS-2.1, etc). Those releases |
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 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
.
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.
Blessed PR
Closes #21438
README.md
from theui
folder.README.md
andCONTRIBUTING.md
Makefile
fetch-ui-assets
script in the/scripts
folder, which seemed like a more appropriate place for it.As you can imagine, the list of deleted file is very long. The deletion of the files is isolated in a single commit, so you should be able to look at the other commits for non-deletion changes.