Skip to content

Allow repositories without an 'origin' remote#353

Merged
derrickstolee merged 4 commits intomicrosoft:masterfrom
derrickstolee:no-origin
Mar 17, 2020
Merged

Allow repositories without an 'origin' remote#353
derrickstolee merged 4 commits intomicrosoft:masterfrom
derrickstolee:no-origin

Conversation

@derrickstolee
Copy link
Contributor

Resolves #350

While playing around with an empty repository, I found that scalar diagnose was failing because it was adding src 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.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
@derrickstolee derrickstolee requested a review from jrbriggs March 17, 2020 15:04
Copy link
Member

@jrbriggs jrbriggs left a 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>
@derrickstolee
Copy link
Contributor Author

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.

Good find. The reference to origin is correct when talking about the GVFS protocol, so I added a sentence about the non-GVFS protocol case and how it will fetch from each remote.

I also figured it would be good to remove that problematic constructor from ScalarEnlistment so we don't hit the src folder thing again. Now the CloneVerb is the only place that knows to add the src directory when creating a ScalarEnlistment.

throw new InvalidRepoException(this.WorkingDirectoryRoot, $"Failed to load remotes with error: {error}");
}

if (remotes.Length > 0)
Copy link
Member

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?

Copy link
Contributor Author

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).

@derrickstolee derrickstolee merged commit c559f5e into microsoft:master Mar 17, 2020
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.

scalar register fails if there is no 'origin' origin

3 participants