-
Notifications
You must be signed in to change notification settings - Fork 351
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
Sysinfo cleanup #1472
Sysinfo cleanup #1472
Conversation
820f47f
to
c87a944
Compare
ee49e70
to
170bdb4
Compare
@@ -34,27 +47,12 @@ jest.mock( | |||
) | |||
|
|||
describe('sysinfo component', () => { | |||
test('should render causal cluster table', () => { |
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.
what's the reason for removing this test?
@@ -0,0 +1,205 @@ | |||
/* |
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.
Could we perhaps use git commands to rename this file? As it stands now, it looks like we'll lose the commit history on this file after it was renamed...
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 was thinking a single commit that only renamed would be enough, I'll try git mv!
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.
Seems to me from this stack overflow thread it doesn't matter if I use git mv instead? https://stackoverflow.com/questions/2314652/is-it-possible-to-move-rename-files-in-git-and-maintain-their-history
@@ -236,41 +236,3 @@ export const responseHandler = (setState: any) => | |||
success: true | |||
}) | |||
} | |||
|
|||
export const clusterResponseHandler = (setState: any) => |
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.
None of this cluster code was used? We should probably check that things are working properly with a server cluster... :-)
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.
In the PR description I've linked the PR that motivated this change :)
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
To understand how the :sysinfo frame works, I've refactored and cleaned it up. This PR doesn't intend change any behaviour.
Changes: