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 usage of FormatUri method #1290

Merged
merged 22 commits into from
May 11, 2016

Conversation

alexander-efremov
Copy link
Contributor

@alexander-efremov alexander-efremov commented May 5, 2016

As part of my work on #1120 I've done some improvements in ApiUrls.cs class.

I've found out that not all clients use ApiUrls to build Uri and get right API endpoint. As far it is directly connected with my work, I fixed these cases.

  • Fix usage of "repos/{0}/{1}".FormatUri(owner, name)
  • Fix usage of "user/keys/{0}".FormatUri(id)
  • Fix usage of "users/{0}".FormatUri(login)
  • Fix usage of "repos/{0}/{1}/stats/contributors".FormatUri(owner, repositoryName)
  • Fix usage of "repos/{0}/{1}/stats/commit_activity".FormatUri(owner, repositoryName)
  • Fix usage of "repos/{0}/{1}/stats/code_frequency".FormatUri(owner, name)
  • Fix usage of "repos/{0}/{1}/stats/participation".FormatUri(owner, name)
  • Fix usage of "repos/{0}/{1}/stats/punch_card".FormatUri(owner, name)
  • Fix usage of "repos/{0}/{1}/collaborators/{2}".FormatUri(owner, repo, user)
  • Fix usage of "orgs/{0}/members/{1}".FormatUri(org, user)
  • Fix usage of "orgs/{0}".FormatUri(organizationName)

/cc @shiftkey, @ryangribble

@alexander-efremov alexander-efremov changed the title Fix usage of FormatUri method. Fix usage of FormatUri method May 5, 2016
@alexander-efremov
Copy link
Contributor Author

Could anybody merge it if there is no problems?

/cc @shiftkey, @ryangribble

fixed usage of "repos/{0}/{1}/stats/contributors".FormatUri(owner, repositoryName)
fixed usage of "repos/{0}/{1}/stats/commit_activity".FormatUri(owner, repositoryName)
fixed usage of "repos/{0}/{1}/stats/code_frequency".FormatUri(owner, name)
fixed usage of "repos/{0}/{1}/stats/participation".FormatUri(owner, name)
fixed usage of "repos/{0}/{1}/stats/punch_card".FormatUri(owner, name)
fixed documentation
fixed usage of "repos/{0}/{1}/collaborators/{2}".FormatUri(owner, repo, user)
fixed usage of"orgs/{0}/members/{1}".FormatUri(org, user)
fixed usage of "orgs/{0}".FormatUri(organizationName)
* Fix inconsistecy in parameters name that used in methods that use "repos/{0}/{1}" syntax.

* added XML documentation for PullRequestFiles method

* updated XML documentation of some methods in ApiUrls

* added needed XML doc tags

* fixed name of parameter

* fixed name of parameter

* undo prev. commit
@alexander-efremov alexander-efremov force-pushed the fix-usage-of-format-uri branch from 8147459 to 23191be Compare May 10, 2016 11:16
@ryangribble
Copy link
Contributor

The builds failed after recent rebase?

@alexander-efremov
Copy link
Contributor Author

alexander-efremov commented May 10, 2016

@ryangribble yes and don't have any suspicions why it failed...

I've figured out - OrganizationMembersClient.cs is missed during rebase I don't know why 😵

@ryangribble
Copy link
Contributor

looks like OrganizationMembersFilter and OrganizationMembersRole are also missing

@alexander-efremov alexander-efremov force-pushed the fix-usage-of-format-uri branch from c8b45b4 to e45f8e0 Compare May 10, 2016 12:13
@alexander-efremov
Copy link
Contributor Author

alexander-efremov commented May 10, 2016

Hope it is fixed in the last commit. Merge/rebase always makes my cry 😢

-

@@ -207,6 +207,10 @@
</CodeAnalysisDictionary>
</ItemGroup>
<ItemGroup>
<ProjectReference Include="..\Octokit\Octokit-Mono.csproj">
Copy link
Member

Choose a reason for hiding this comment

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

This reference isn't necessary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, fixed!

@@ -1,6 +1,10 @@
using System;
using System.Collections.Generic;
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure why these using statements are needed

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 removed all unused usings in the file.

@shiftkey
Copy link
Member

@dampir a couple of unnecessary introduced changes, but aside from that this looks good. ❤️ seeing the inlined URLs being extracted!

@alexander-efremov
Copy link
Contributor Author

alexander-efremov commented May 10, 2016

seeing the inlined URLs being extracted!

Without it I can't properly generate new urls to access API by repo id 😄
Hope it would be merged as far as possible in order I can pull out these new urls.

@shiftkey shiftkey merged commit efa2942 into octokit:master May 11, 2016
@alexander-efremov alexander-efremov deleted the fix-usage-of-format-uri branch May 11, 2016 07:05
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.

3 participants