-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Remove all demo-mode demos #1645
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary: See #1640. This removes all demos that instantiate a router in demo mode. None of these demos worked: I loaded each one manually, and each one failed to load some required resources. Demos in `plugins/graph` and `plugins/interactive_inference` have been allowed to stay, because (a) they actually work to at least some degree, and (b) they do not use our router’s demo mode so are not posing any obstacles. For ease of review, this commit is a mass-deletion only. A separate commit will include the structural changes that need to be made to actually remove demo-mode. Test Plan: Check that routers are only created in the `tf_backend` implementation and tests: $ git grep --name-only createRouter tensorboard/components/tf_backend/router.ts tensorboard/components/tf_backend/test/backendTests.ts Note that all existing tests pass in Python 2 and 3. wchargin-branch: remove-demos
Here are some weird traces of demo ➜ tensorboard git:(master) ✗ grep -r "@demo" tensorboard/*
tensorboard/components/vz_chart_helpers/vz-chart-tooltip.html:@demo demo/index.html
tensorboard/components/vz_line_chart/vz-line-chart.html:@demo demo/index.html
tensorboard/components/vz_line_chart2/vz-line-chart2.html:@demo demo/index.html
tensorboard/plugins/graph/tf_graph_app/tf-graph-app.html:@demo demo/index.html
tensorboard/plugins/profile/google_chart/google-chart.html:@demo
tensorboard/plugins/histogram/vz_histogram_timeseries/vz-histogram-timeseries.html:@demo demo/index.html Seems like google_chart one should remain the same though. |
stephanwlee
approved these changes
Nov 29, 2018
Thanks! I didn’t know about those
Will handle these as listed above unless you disagree. Unrelated: my Travis build took 40 minutes to start. |
wchargin
added a commit
that referenced
this pull request
Nov 29, 2018
Summary: Some modules have `@demo` annotations that point to nonexistent demos, so we fix or remove those references as appropriate. The `google-chart` demo is not working and does not appear to have ever worked: when running it on its initial commit after cherry-picking #1334 we see only a blank page. However, deleting this demo isn’t trivial, because the main profile dashboard depends on it for some reason. As this plugin is still being actively developed, we’ll leave the demo alone for now: it’s not actually causing any problems. Context: <#1645 (comment)> Test Plan: Note that `git grep @demo` only lists correct paths to working demos. wchargin-branch: remove-broken-demos
wchargin
added a commit
that referenced
this pull request
Nov 29, 2018
Summary: Some modules have `@demo` annotations that point to nonexistent demos, so we fix or remove those references as appropriate. The `google-chart` demo is not working and does not appear to have ever worked: when running it on its initial commit after cherry-picking #1334 we see only a blank page. However, deleting this demo isn’t trivial, because the main profile dashboard depends on it for some reason. As this plugin is still being actively developed, we’ll leave the demo alone for now: it’s not actually causing any problems. Context: <#1645 (comment)> Test Plan: Note that `git grep @demo` only lists correct paths to working demos. wchargin-branch: remove-broken-demos
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
See #1640. This removes all demos that instantiate a router in demo
mode. None of these demos worked: I loaded each one manually, and each
one failed to load some required resources.
Demos in
plugins/graph
andplugins/interactive_inference
have beenallowed to stay, because (a) they actually work to at least some degree,
and (b) they do not use our router’s demo mode so are not posing any
obstacles.
For ease of review, this commit is a mass-deletion only. A separate
commit will include the structural changes that need to be made to
actually remove demo-mode.
Test Plan:
Check that routers are only created in the
tf_backend
implementationand tests:
Note that all existing tests pass in Python 2 and 3.
wchargin-branch: remove-demos