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

mgr/dashboard: Enable object rendering in KV-table #21701

Merged
merged 1 commit into from May 2, 2018

Conversation

Devp00l
Copy link

@Devp00l Devp00l commented Apr 27, 2018

Now it's possible to render objects as values in our key-value table.

Signed-off-by: Stephan Müller smueller@suse.com

this.tableData = this.makePairs(this.data);
}

makePairs(data: any) {
Copy link
Member

Choose a reason for hiding this comment

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

Consider to make this method private.

return this.postProcessing(temp);
}

makePairsFromArray(data: any[]) {
Copy link
Member

Choose a reason for hiding this comment

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

Consider to make this method private.

return temp;
}

makePairsFromObject(data: object) {
Copy link
Member

Choose a reason for hiding this comment

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

Consider to make this method private.

}));
}

postProcessing(data: any[]) {
Copy link
Member

Choose a reason for hiding this comment

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

Please add a method description about what it is doing and why.

Copy link
Author

@Devp00l Devp00l May 2, 2018

Choose a reason for hiding this comment

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

I've renamed it to make clear what it does

}));
}

postProcessing(data: any[]) {
Copy link
Member

Choose a reason for hiding this comment

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

Consider to make this method private.

return temp;
}

convertValue(v: any) {
Copy link
Member

Choose a reason for hiding this comment

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

Consider to make this method private.

Copy link
Author

Choose a reason for hiding this comment

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

I will rename them with an underscore instead so that they are still testable.

Copy link
Contributor

@ricardoasmarques ricardoasmarques left a comment

Choose a reason for hiding this comment

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

lgtm

Now it's possible to render objects as values in our key-value table.

Signed-off-by: Stephan Müller <smueller@suse.com>
@Devp00l Devp00l force-pushed the wip-kv-table-render-object branch from 968b8fe to 98fc5f9 Compare May 2, 2018 10:10
@Devp00l
Copy link
Author

Devp00l commented May 2, 2018

@votdev I've addressed all your comments

@LenzGr
Copy link
Contributor

LenzGr commented May 2, 2018

retest this please

@LenzGr LenzGr merged commit c608185 into ceph:master May 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants