Skip to content

Only report log collection progress in sourcekit-lsp diagnose if we’ve made meaningful progress #1432

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

Merged
merged 1 commit into from
Jun 6, 2024

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Jun 5, 2024

Especially when we didn’t have a redrawing progress indicator, we would spam the output with out making progress that was visible to the user because the overall progress just shows a percentage without any decimal digits. To fix this, only report progress when we’ve made at least 1% of progress in log collection.

@ahoppen ahoppen requested a review from benlangmuir as a code owner June 5, 2024 18:18
@ahoppen ahoppen requested review from bnbarham and hamishknight June 5, 2024 18:19
@ahoppen
Copy link
Member Author

ahoppen commented Jun 5, 2024

@swift-ci Please test

Comment on lines 226 to 228
if progress - lastReportedProgress > 0.01 {
// Only call `reportProgress` when we've made at least 1% of progress (which will get scaled down further
// based on the other components). This avoids us spamming the console with non-meaningful progress updates.
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO it seems like this is something that should be handled by reportProgress, but I don't feel strongly about it

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that’s probably the better design. Updated.

…’ve made meaningful progress

Especially when we didn’t have a redrawing progress indicator, we would spam the output with out making progress that was visible to the user because the overall progress just shows a percentage without any decimal digits. To fix this, only report progress when we’ve made at least 1% of progress in log collection.
@ahoppen ahoppen force-pushed the dont-spam-log-progress branch from 336f3d3 to 6425a8e Compare June 5, 2024 21:34
@ahoppen
Copy link
Member Author

ahoppen commented Jun 5, 2024

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Jun 5, 2024

@swift-ci Please test Windows

@ahoppen ahoppen merged commit 9629247 into swiftlang:main Jun 6, 2024
3 checks passed
@ahoppen ahoppen deleted the dont-spam-log-progress branch June 6, 2024 04:35
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.

2 participants