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

[quality][performance] Format performance numbers in the log #6858

Merged
merged 1 commit into from
Jan 13, 2020
Merged

[quality][performance] Format performance numbers in the log #6858

merged 1 commit into from
Jan 13, 2020

Conversation

kaiyue0329
Copy link
Contributor

Issue: #6531

What it does

  • formatted the performance numbers in the log to one decimal place.

Review checklist

Reminder for reviewers

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

It fixes the issue 👍

I'm wondering, what do you think of having a utility method in @theia/core (the base extension), which would accept as arguments content and milliseconds which can be re-used when displaying metric results? This would mean that we would not need to re-implement the same code in different places with the same purpose.

Something like:

export function printMetric(content: string, milliseconds: number): void {
    console.log(`${content} took ${Math.round(milliseconds * 10) / 10} ms`);
}

@akosyakov akosyakov added performance issues related to performance quality issues related to code and application quality labels Jan 10, 2020
- formatted the performance numbers in the log to one decimal place.

Signed-off-by: Kaiyue Pan <kaiyue.pan@ericsson.com>
@kittaakos
Copy link
Contributor

would not need to re-implement the same code

We could use ms.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance issues related to performance quality issues related to code and application quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants