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

Displaying a prompt when user clones repository that exists locally #59714

Closed

Conversation

rdosanjh
Copy link

@rdosanjh rdosanjh commented Oct 1, 2018

Fixes #48490

vscode

This stores the fetchUrl, repository Location and last updated in the global store. If the amount in the store hits 50 it will empty the least recently updated repos.

When cloning a repo it will look up the fetch url on the list in the global store and then check that the directory exists.

I started by using a map object using the fetchUrl as a key for fast lookup. This increased the amount of code needed to limit the amount to 50. I decided that for 50 records a map and an array wouldn't be too different in performance.

Let me know what you think of the approach.

I spent some time trying to write tests around this but i was finding it difficult to mock the Repository class from git.ts. I was going to extract an interface and create a Mock implementation but as the area isn't well covered i decided i should talk to somebody on the team before going ahead with that.

@joaomoreno joaomoreno added this to the October 2018 milestone Oct 1, 2018
@joaomoreno joaomoreno added the git GIT issues label Oct 1, 2018
@joaomoreno joaomoreno modified the milestones: October 2018, November 2018 Nov 6, 2018
@joaomoreno joaomoreno modified the milestones: November 2018, Backlog Dec 3, 2018
Copy link
Member

@joaomoreno joaomoreno left a comment

Choose a reason for hiding this comment

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

Sorry for the late review.

Quite some issues with this PR:

  • Use of sync Node API is a no-no
  • Lack of explicit typing... we need to type every function's return value, for example
  • Lack of code style preservation
  • Confusing return types, eg getRepositoryLocation returns boolean | string
  • Updating the knownRepositories structure occurred in one place, while reading from it occurred in another place.
  • The slicing of the knownRepositories array occurred after reading it from storage instead of before writing it to storage. Also, sorting it all the time is irrelevant if we simply use the array as a queue.
  • KnownRepository should not be declared in the public API git.d.ts file
  • What if only pushUrl is defined in a remote?
  • What if a remote is referenced in two separate local repositories?

@joaomoreno joaomoreno closed this May 24, 2019
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
git GIT issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Git: Clone command should find repository, if known
2 participants