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

Git fetch improvement #1511

Merged
merged 2 commits into from
Dec 16, 2023
Merged

Conversation

Wouter01
Copy link
Member

Description

A small improvement for git fetching.

Previously, a timer was used to fetch the repo when the source control sidebar pane is active. This timer launches a new task, but doesn't keep track of it. When a fetch takes more than 10 seconds, the timer will start fetching again, but the previous fetch won't stop. Moreover, Task implicitly captures self, which is problematic in this case.

The periodic refresh function has been removed. Instead, a task is created in the view, and will repeatedly call the fetch method. The task gets cancelled when the view disappears.

When the fetch fails, an error message should be displayed. This wasn't handled previously, and isn't now, but a todo has been added and the try has been moved in a do block, instead of failing silently. (I'm assuming the error should be displayed in some kind of notification panel, which doesn't exist atm)

Additionally, an unneeded task has been removed.

Related Issues

None

Checklist

  • I read and understood the contributing guide as well as the code of conduct
  • The issues this PR addresses are related to each other
  • My changes generate no new warnings
  • My code builds and runs on my machine
  • My changes are all related to the related issue above
  • I documented my code

@Wouter01 Wouter01 added the bug Something isn't working label Dec 15, 2023
Copy link
Collaborator

@thecoolwinter thecoolwinter left a comment

Choose a reason for hiding this comment

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

Good improvements 👍

@Wouter01 Wouter01 merged commit 8bd8edc into CodeEditApp:main Dec 16, 2023
2 checks passed
@austincondiff
Copy link
Collaborator

Thanks for the PR, good catch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants