Introduce Remote.IsSupportedUrl() #754
Conversation
| @@ -304,5 +304,19 @@ public void CanNotRenameWhenRemoteWithSameNameExists() | |||
| Assert.Throws<NameConflictException>(() => repo.Network.Remotes.Rename("origin", "upstream")); | |||
| } | |||
| } | |||
|
|
|||
| [Theory] | |||
| [InlineData("[email protected]:org/repo")] | |||
There was a problem hiding this comment.
I can change the test data to be somewhat more generic, since git, http, https, ftp, ftps have the same standard: protocol://site.com:port/path/to/repo.git. But I guess having them like this doesn't hurt.
There was a problem hiding this comment.
Shouldn't we also check that this url is supported?
There was a problem hiding this comment.
Oh, that sounds like a great idea!
There was a problem hiding this comment.
So, should I pack the IsValidUrl to check valid_url and supported_url?
There was a problem hiding this comment.
Added support for IsSupportedUrl.
There was a problem hiding this comment.
I'm not sure I think we should expose both IsValidUrl() and IsSupportedUrl().
Indeed, from a user perspective, knowing that an url is valid may be of little value if it's not supported by the underlying library.
I'd be 👍 to drop IsValidUrl()
There was a problem hiding this comment.
Dropped IsValidUrl due to IsSupportedUrl being a more enforced version of it.
|
Awesome! Just a little nitpick before merging this. Could you please reword the commit message? |
|
Done. |
|
🎱 Thanks! |
nulltoken
changed the title
No description provided.