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

StatisticsClient should not clobber /api/v3/ in path #1085

Merged
merged 1 commit into from
Feb 4, 2016

Conversation

shiftkey
Copy link
Member

@shiftkey shiftkey commented Feb 3, 2016

Fixes #822

All other routes - e.g. ApiUrls - are defined without a leading slash.

Except these. This fixes that.

  • fix the tests - you're better than this

@shiftkey shiftkey changed the title StatisticsClient should not clobber /api/v3/ in path [WIP] StatisticsClient should not clobber /api/v3/ in path Feb 3, 2016
@shiftkey shiftkey force-pushed the relative-paths-statistics branch from 3ca48cb to 3d7a20c Compare February 3, 2016 00:53
@shiftkey shiftkey changed the title [WIP] StatisticsClient should not clobber /api/v3/ in path StatisticsClient should not clobber /api/v3/ in path Feb 3, 2016
@shiftkey
Copy link
Member Author

shiftkey commented Feb 3, 2016

Gotta rebase once #1087 is merged

… slash).

This was one of the cases we didn't notice, and so it causes the
StatisticsClient to drop the /api/v3/ prefix that GitHub Enterprise
requires to route things correctly.
@shiftkey shiftkey force-pushed the relative-paths-statistics branch from 3d7a20c to 4384992 Compare February 3, 2016 02:00
@shiftkey
Copy link
Member Author

shiftkey commented Feb 4, 2016

Ready for review

@haacked
Copy link
Contributor

haacked commented Feb 4, 2016

Nice catch!

haacked added a commit that referenced this pull request Feb 4, 2016
StatisticsClient should not clobber /api/v3/ in path
@haacked haacked merged commit deb61c7 into master Feb 4, 2016
@haacked haacked deleted the relative-paths-statistics branch February 4, 2016 00:46
@haacked
Copy link
Contributor

haacked commented Feb 4, 2016

We should do a Roslyn analyzer to not allow absolute Uri in Octokit.net since I can't think of any place where they would be allowed. We can always suppress in those cases where they're necessary.

(attempted nerd snipe)

@shiftkey
Copy link
Member Author

shiftkey commented Feb 4, 2016

(attempted nerd snipe)

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.

StatisticsClient isn't Enterprise-friendly
2 participants