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

Simplify and optimize util.TimeTrack #4924

Merged
merged 1 commit into from
May 18, 2023
Merged

Simplify and optimize util.TimeTrack #4924

merged 1 commit into from
May 18, 2023

Conversation

gilbertsoft
Copy link
Member

@gilbertsoft gilbertsoft commented May 17, 2023

The Issue

The util.TimeTrack() function has currently a parameter start which was always set to time.Now() and can therefor set directly in the function instead which will reduce the impact a bit and simplify the usage. Also in many cases the name parameter was manually set to the calling function name which is also error prone.

With this patch, the TimeTrack functionallity was moved to a dedicated file in the same package and two functions are now available, one without parameters where the calling function name is retrieved with the help of the runtime package and a second with a name parameter to individually set a name e.g. used for tests. Source documentation was extended to also show a simpler one line usage which is apropriate in most cases. Also some minor optimization are done like avoiding the expensive format functions e.g. Printf. Finally the outputed messages are streamlined to increase visibility.

@github-actions
Copy link

github-actions bot commented May 17, 2023

@gilbertsoft gilbertsoft marked this pull request as ready for review May 17, 2023 20:07
@gilbertsoft gilbertsoft requested a review from a team as a code owner May 17, 2023 20:07
Copy link
Member

@rfay rfay left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for improving this. And I forget this feature even exists, it will be useful for studying my worries about mutagen performance.

@rfay
Copy link
Member

rfay commented May 18, 2023

Also thanks for separating this out into its own PR. It's always great for PRs to have their own specific purpose.

@rfay rfay merged commit cf5bf15 into master May 18, 2023
17 checks passed
@rfay rfay deleted the task/improve-time-track branch May 18, 2023 02:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants