Skip to content

Make update check faster#249

Open
jxstxn1 wants to merge 5 commits intophntmxyz:mainfrom
jxstxn1:make-update-check-faster
Open

Make update check faster#249
jxstxn1 wants to merge 5 commits intophntmxyz:mainfrom
jxstxn1:make-update-check-faster

Conversation

@jxstxn1
Copy link
Copy Markdown
Contributor

@jxstxn1 jxstxn1 commented Oct 10, 2023

This PR is addressing this issue #174

Starting the update check at the beginning of a command in an isolate and execute the returned function, which contains the _checkCliVersionIntegrity and may contain the print that an update is available

Comment thread sidekick_core/lib/sidekick_core.dart Outdated
Comment on lines 334 to 336
void Function()? updateCheck;
try {
final updateFuture = VersionChecker.isDependencyUpToDate(
Copy link
Copy Markdown
Contributor

@passsy passsy Oct 16, 2023

Choose a reason for hiding this comment

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

The try catch is now useless, because the updateFuture is not awaited.
The error of VersionChecker.isDependencyUpToDate() is never caught and would crash the CLI

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The Future itself is awaited in Line 345 in combination with the timeout
Errors are caught and result in returning null

Comment thread sidekick_core/lib/sidekick_core.dart Outdated
final unmount = mount(debugName: args.join(' '));

ArgResults? parsedArgs;
Future<void Function()?>? updateCheck;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A Future<void> should be sufficient. The void Function() seems redundant

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I updated it to a String? :)

Justin Baumann added 2 commits November 4, 2023 15:15
@jxstxn1 jxstxn1 requested a review from passsy January 6, 2024 11:20
@passsy passsy force-pushed the main branch 4 times, most recently from 993b9d6 to 632af79 Compare June 30, 2025 14:50
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