Skip to content
This repository was archived by the owner on Nov 20, 2018. It is now read-only.

Rename UriHelper.Encode #648

Merged
merged 1 commit into from
Jun 6, 2016
Merged

Rename UriHelper.Encode #648

merged 1 commit into from
Jun 6, 2016

Conversation

Tratcher
Copy link
Member

@Tratcher Tratcher commented Jun 3, 2016

#573 @NTaylorMullen @muratg @strohhut

Also clean up the sample project so it will run again.

@Tratcher Tratcher added this to the 1.0.0 milestone Jun 3, 2016
@Tratcher Tratcher self-assigned this Jun 3, 2016
@@ -80,7 +80,7 @@ public static string Encode(Uri uri)
{
if (uri.IsAbsoluteUri)
{
return Encode(
return BuildAbsolute(
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious: Why is there not a corresponding BuildRelative call in the else?

Copy link
Member Author

Choose a reason for hiding this comment

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

System.Uri has minimal support for relative uris. It won't parse it into path?query#fragment for you. The most it will do is some basic escaping.

@NTaylorMullen
Copy link
Contributor

The Encode => BuildAbsolute rename is good. Had a few comments about the API exposed in UriHelper, we can talk offline about them if it's easier.


namespace SampleApp
{
public class Program
public static class Program
Copy link

Choose a reason for hiding this comment

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

Nit: no need for static

@Tratcher
Copy link
Member Author

Tratcher commented Jun 6, 2016

Updated.

"dnxcore50"
],
"dependencies": {
"System.Console": "4.0.0-*",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you need this? I think it's pulled in via Microsoft.NETCore.App via NETStandard.Library.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, removed.

@NTaylorMullen
Copy link
Contributor

:shipit:

@Tratcher Tratcher force-pushed the tratcher/urihelper branch from 393ff46 to 3fc1fef Compare June 6, 2016 20:30
@Tratcher Tratcher merged commit 3fc1fef into dev Jun 6, 2016
@Tratcher Tratcher deleted the tratcher/urihelper branch June 6, 2016 20:31
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.

6 participants