Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Conversation

haacked
Copy link
Contributor

@haacked haacked commented Jul 30, 2015

Fixes #13

This PR not only fixes #13, but consolidates all the URL parsing of repository information by leveraging the UriString class adapted from GHfW.

haacked added 10 commits July 30, 2015 15:04
Note that ToUri isn't used anywhere, so I replaced it.
Sadly we can't just do this: new Uri(url, path); because you get
different results if the url ends with "/" or not. So we need to make
absolutely sure it _does_ end with "/".
We no longer need this.
Fixes #13

We need to preserve the format of the origin for non github.com
repositories (such as GitHub Enterprise instances). Many of them are not
set up with HTTPS because they're behind a firewall etc.
We have some places where we used custom parsing logic for extracting
owner and repo from a repository URL. Instead, we should use the robust
logic from UriString.
I noticed this was no longer included in the project. Probably should
be.
This method name makes more sense.
@haacked haacked force-pushed the haacked/13-handle-http-urls branch from 5112ae4 to 56add34 Compare July 30, 2015 22:04
Let's not go overboard with C# 6
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there an advantage to doing this with the value type instead of the class type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope. I think it's my overeager R# that does this. We should pick an approach and stick to it. Do you have a preference?

BTW, this has nothing to do with value type or class type. string is just the shorthand for System.String but it's the exact same class. 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

Nope. I think it's my overeager R# that does this. We should pick an approach and stick to it. Do you have a preference?

Weeell, I always type String.Something out of habit, so I'm sure the code is way more sprinkled with the uppercase version than the lowercase one (and I doubt I can teach my fingers otherwise...) 😄

BTW, this has nothing to do with value type or class type. string is just the shorthand for System.String but it's the exact same class. 😄

Yeah, I keep forgetting we're in .NET land where there are no crazy peeps doing weird stuff with jit and string/String differences...

@haacked
Copy link
Contributor Author

haacked commented Jul 31, 2015

🍍

shana added a commit that referenced this pull request Aug 1, 2015
@shana shana merged commit e021c32 into master Aug 1, 2015
@shana shana deleted the haacked/13-handle-http-urls branch August 1, 2015 03:43
@shana
Copy link
Contributor

shana commented Aug 1, 2015

🍨

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GitHub Enterprise Repo Links are always SSL

2 participants