-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Update user-facing terminology in the vtctld2 UI #6481
Conversation
Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com>
Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com>
I don't remember why I decided to run |
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.
Stop Slave
was a reference to the actual mysql command and made sense, but it is better to now refer to it as Start/Stop/starting/stopping Replication
because you are not stopping or starting the whole instance or process, just the replication stream. This is also what we did on the server side.
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.
LGTM.
Agree with Deepthi: any regressions due to people having modified the bundles directly can be fixed "properly" once they are discovered!
Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com>
Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com>
Sorry it took so long to apply your suggestions, @deepthi! I blame Docker + my terrible internet. 😂 Here's what it looks like now: It might be a good idea to squash merge this PR. 🎃 I've been committing the .ts file changes + the generated assets in separate commits to tidy up the commit history; thinking in terms of rollbacks + cherry picking, it probably makes the most sense to treat this PR as an atomic unit. Up to you though! |
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.
LGTM
All tests are showing green in the Actions view. |
...because I'm a little bit cowardly of making logic changes to an unfamiliar UI, I figured it was less risky to only change the display values and leave the Typescript alone (for now). Unfortunately, this leaves a handful of instances of oppressive terminology in the code base, but they won't be user-facing.
The static files and rice-box.go changes were auto-generated with the new build tooling.
One thing worth noting is that this overwrites the files to go/vt/vtctld/rice-box.go that were made in 1acb6e0. Trying to parse that diff is an absolute nightmare, so I was unsuccessful in copying over the changes. 😞 I don't see any updates to the unbuilt
web/vtctld2/
files that might suggest what the changes were -- @deepthi do you happen to remember what they were, and if I should copy them over?I also updated the copyright date in the new FE build scripts per @rohit-nayak-ps's suggestion on #6473. :)
Here's one example of the UI before vs. after: