-
Notifications
You must be signed in to change notification settings - Fork 63
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
Allow repositories without an 'origin' remote #353
Conversation
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with suggestion: While we're here, let's clean up the documentation about background fetch, which still makes mention of origin
, even though it actually iterates your remotes without regard to what they're named.
…out URL Remove the other constructor that allows this by default to avoid this problem in the future. Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
5923279
to
d09d345
Compare
Good find. The reference to I also figured it would be good to remove that problematic constructor from |
throw new InvalidRepoException(this.WorkingDirectoryRoot, $"Failed to load remotes with error: {error}"); | ||
} | ||
|
||
if (remotes.Length > 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be remotes.Length == 1
? If there are more that one are we sure we are doing the right thing to just take the first remote? Should there be a config setting for that case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought it safer to have a RepoUrl setting rather than have none.
I think it is important that we will not support multiple remotes when using the GVFS protocol, and I think that's the only time we use RepoUrl (other than logging).
Resolves #350
While playing around with an empty repository, I found that
scalar diagnose
was failing because it was addingsrc
to the working directory. Turns out there is a missed path that happens depending on the verb options, and now it is fixed.While doing that, improve the message for
InvalidRepoException
.