Skip to content
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

Fix for Upload to Github releases stopped working #894 #895

Merged
merged 4 commits into from
Sep 17, 2015

Conversation

naveensrinivasan
Copy link

Fix for Upload to Github releases stopped working #894. The ExpandUriTemplate could handle only one parameter which is the bug.

Naveen added 2 commits September 15, 2015 19:07
Fix for Upload to Github releases stopped working octokit#894. The
ExpandUriTemplate could handle only one parameter which is the bug.
@@ -57,9 +57,11 @@ public class TheExpandUriTemplateMethod
[InlineData("https://host.com/path?name=other", "https://host.com/path?name=other")]
[InlineData("https://host.com/path?name=example name.txt", "https://host.com/path{?name}")]
[InlineData("https://host.com/path", "https://host.com/path{?other}")]
[InlineData("https://host.com/path?name=example name.txt,label=labeltext", "https://host.com/path{?name,label}")]
[InlineData("https://host.com/path?name=example name.txt,label=labeltext", "https://host.com/path{?name,label,other}")]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is that the proper expansion? Or should it be ?name=example name.txt&label=labeltext

Naveen added 2 commits September 16, 2015 19:26
It should have been "&" instead of "," for the URL for the issue octokit#844
Update StringExtensionsTests.cs  for the URL expansion. The initial fix was incorrect.
@haacked
Copy link
Contributor

haacked commented Sep 16, 2015

🆒 Did you test this out?

@naveensrinivasan
Copy link
Author

Yes, I checked in the test before the actual fix that was causing the CI to fail the build.

@haacked
Copy link
Contributor

haacked commented Sep 16, 2015

Yes, I checked in the test before the actual fix that was causing the CI to fail the build.

Ok, I just wanted to make sure you actually tested the Upload to Releases with this fix.

@naveensrinivasan
Copy link
Author

Yes, I tested the Upload to releases with this fix.

https://github.com/naveensrinivasan/TestUpload/releases

@haacked
Copy link
Contributor

haacked commented Sep 17, 2015

selfie-0

@haacked
Copy link
Contributor

haacked commented Sep 17, 2015

Thanks!!!

haacked added a commit that referenced this pull request Sep 17, 2015
Fix for Upload to Github releases stopped working #894
@haacked haacked merged commit 32c154f into octokit:master Sep 17, 2015
@forki
Copy link
Contributor

forki commented Sep 17, 2015

Very cool. Are you going to release this as a hot fix? It's blocking the whole F# ecosystem at the moment ;-)

naveensrinivasan pushed a commit to naveensrinivasan/octokit.net that referenced this pull request Sep 17, 2015
Merge pull request octokit#895 from naveensrinivasan/master
@haacked
Copy link
Contributor

haacked commented Sep 17, 2015

It's blocking the whole F# ecosystem at the moment

Yikes! I'll try and get a release out soon.

@forki
Copy link
Contributor

forki commented Sep 17, 2015

Yes. Thanks to projectscaffold project everybody uses Octokit. ;-)

Thanks again to both of you. Really appreciate this.
On Sep 17, 2015 8:07 PM, "Phil Haack" notifications@github.com wrote:

It's blocking the whole F# ecosystem at the moment

Yikes! I'll try and get a release out soon.


Reply to this email directly or view it on GitHub
#895 (comment).

@haacked
Copy link
Contributor

haacked commented Sep 17, 2015

@forki cool, got a sec to sanity check the release notes here? #898

@forki
Copy link
Contributor

forki commented Sep 17, 2015

Lol. Made my day.
On Sep 17, 2015 21:56, "Phil Haack" notifications@github.com wrote:

@forki https://github.com/forki cool, got a sec to sanity check the
release notes here? #898 #898


Reply to this email directly or view it on GitHub
#895 (comment).

@haacked
Copy link
Contributor

haacked commented Sep 17, 2015

This is now live on NuGet as of version 0.16.0!

@forki
Copy link
Contributor

forki commented Sep 21, 2015

works like a charme. thanks so much.

free

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.

None yet

3 participants