{"action":"created","issue":{"url":"https://api.github.com/repos/shiftkey/Octokit.CodeAnalysis/issues/1","labels_url":"https://api.github.com/repos/shiftkey/Octokit.CodeAnalysis/issues/1/labels{/name}","comments_url":"https://api.github.com/repos/shiftkey/Octokit.CodeAnalysis/issues/1/comments","events_url":"https://api.github.com/repos/shiftkey/Octokit.CodeAnalysis/issues/1/events","html_url":"https://github.com/shiftkey/Octokit.CodeAnalysis/pull/1","id":122876874,"number":1,"title":"[WIP] Auditing the GitHub API using Analyzers","user":{"login":"shiftkey","id":359239,"avatar_url":"https://avatars.githubusercontent.com/u/359239?v=3","gravatar_id":"","url":"https://api.github.com/users/shiftkey","html_url":"https://github.com/shiftkey","followers_url":"https://api.github.com/users/shiftkey/followers","following_url":"https://api.github.com/users/shiftkey/following{/other_user}","gists_url":"https://api.github.com/users/shiftkey/gists{/gist_id}","starred_url":"https://api.github.com/users/shiftkey/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/shiftkey/subscriptions","organizations_url":"https://api.github.com/users/shiftkey/orgs","repos_url":"https://api.github.com/users/shiftkey/repos","events_url":"https://api.github.com/users/shiftkey/events{/privacy}","received_events_url":"https://api.github.com/users/shiftkey/received_events","type":"User","site_admin":true},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":2,"created_at":"2015-12-18T04:19:47Z","updated_at":"2015-12-18T04:41:07Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/shiftkey/Octokit.CodeAnalysis/pulls/1","html_url":"https://github.com/shiftkey/Octokit.CodeAnalysis/pull/1","diff_url":"https://github.com/shiftkey/Octokit.CodeAnalysis/pull/1.diff","patch_url":"https://github.com/shiftkey/Octokit.CodeAnalysis/pull/1.patch"},"body":"I found myself working through the excellent work that @naveensrinivasan had done to parse the GitHub API docs and identify which endpoints had not been implemented in Octokit.\r\n\r\nhttps://github.com/octokit/octokit.net/issues/968#issuecomment-163066920\r\n\r\nAnd sure, I could audit these manually - in fact I started doing this, but then I paused to think about how I could automate this, so that it could be done more than once.\r\n\r\nThis PR starts the first step of creating that auditing process. Let me explain what I'm trying to do here...\r\n\r\n### The Current State Of Affairs\r\n\r\nLet's say we have a method that does this:\r\n\r\n```csharp\r\npublic Task<IReadOnlyList<string>> GetAllForRepository(string owner, string name)\r\n{\r\n    Ensure.ArgumentNotNullOrEmptyString(owner, \"owner\");\r\n    Ensure.ArgumentNotNullOrEmptyString(name, \"name\");\r\n\r\n    return ApiConnection.GetAll<string>(ApiUrls.IssuesEvent(owner, name));\r\n}\r\n```\r\n\r\nIt's not easy to see what resource this is actually associated with - I need to navigate to `ApiUrls` and look at the implementation:\r\n\r\n```csharp\r\npublic static Uri IssuesEvent(string owner, string name)\r\n{\r\n     return \"repos/{0}/{1}/issues/events\".FormatUri(owner, name);\r\n}\r\n```\r\n\r\nI also need some familiarity with the GitHub API docs to ensure that this is actually the right format (arguments are added, in the correct order).\r\n\r\nThis is also just a convenience method. I'm not sure it adds much these days.\r\n\r\n### The Problem\r\n\r\nLet's say I have a page of resources that I extracted from the GitHub API docs:\r\n\r\n```\r\n/activity/events/\r\n\r\n - GET /events\r\n - GET /repos/:owner/:repo/events\r\n - GET /repos/:owner/:repo/issues/events\r\n - GET /networks/:owner/:repo/events\r\n - GET /orgs/:org/events\r\n - GET /users/:username/received_events\r\n - GET /users/:username/received_events/public\r\n - GET /users/:username/events\r\n - GET /users/:username/events/public\r\n - GET /users/:username/events/orgs/:org\r\n```\r\n\r\nThis is just the interesting bits after you parse this page:\r\n\r\nhttps://developer.github.com/v3/activity/events/\r\n\r\nIf I wanted to be in sync with the GitHub API, I'd like for all of these actions to exist on one of the `Clients` defined here - but how could I verify this programmatically? \r\n\r\n### An Alternative Proposal \r\n\r\nBroadly speaking, there's two things I'd like to verify:\r\n\r\n - for a documented feature (e.g. `/activity/events/`) I'd like to verify that a number of methods are implemented\r\n - for each method, verify that it's actually invoking that resource\r\n\r\nSo let's :fire: the `ApiUrls` class to the ground as that's getting in the way, and move these resources closer to where they're needed.\r\n\r\nI'm imagining something like this for defining the contract:\r\n\r\n```csharp\r\n[Endpoint(\"repos/:owner/:repo/issues/events\")]\r\npublic Task<IReadOnlyList<string>> GetAllForRepository(string owner, string name)\r\n{\r\n    Ensure.ArgumentNotNullOrEmptyString(owner, \"owner\");\r\n    Ensure.ArgumentNotNullOrEmptyString(name, \"name\");\r\n\r\n    var uri = \"repos/{0}/{1}/issues/events\".FormatUri(owner, name);\r\n\r\n    return ApiConnection.GetAll<string>(uri);\r\n}\r\n```\r\n\r\nFor other scenarios, you may want to specify the verb as well:\r\n\r\n```csharp\r\n[Endpoint(\"GET\", \"repos/:owner/:repo/issues/events\")]\r\npublic Task<IReadOnlyList<string>> GetAllForRepository(string owner, string name)\r\n{\r\n    Ensure.ArgumentNotNullOrEmptyString(owner, \"owner\");\r\n    Ensure.ArgumentNotNullOrEmptyString(name, \"name\");\r\n\r\n    var uri = \"repos/{0}/{1}/issues/events\".FormatUri(owner, name);\r\n\r\n    return ApiConnection.GetAll<string>(uri);\r\n}\r\n```\r\n\r\nBenefits:\r\n\r\n - can compare attribute and defined endpoint at compile time \r\n - can inspect `ApiConnection` usages matches documented behaviour\r\n - can use reflection to verify API coverage after build (not done here)\r\n - can be introduced gradually by opt-in\r\n - becomes an IDE integration available to anyone on VS2015\r\n\r\nProblems:\r\n\r\n - duplication of resources?\r\n - general inexperience with Roslyn interfering with the contributing experience\r\n\r\nTo Do List:\r\n\r\n - [ ] actually implement a failing test\r\n - [ ] fail the test if the parameters don't match\r\n - [ ] fail the test if the parameters are out of order\r\n - [ ] get some feedback from people more experienced with Roslyn\r\n\r\ncc @pilchie @haacked @M-Zuber for general feedback\r\n"},"comment":{"url":"https://api.github.com/repos/shiftkey/Octokit.CodeAnalysis/issues/comments/165672155","html_url":"https://github.com/shiftkey/Octokit.CodeAnalysis/pull/1#issuecomment-165672155","issue_url":"https://api.github.com/repos/shiftkey/Octokit.CodeAnalysis/issues/1","id":165672155,"user":{"login":"shiftkey","id":359239,"avatar_url":"https://avatars.githubusercontent.com/u/359239?v=3","gravatar_id":"","url":"https://api.github.com/users/shiftkey","html_url":"https://github.com/shiftkey","followers_url":"https://api.github.com/users/shiftkey/followers","following_url":"https://api.github.com/users/shiftkey/following{/other_user}","gists_url":"https://api.github.com/users/shiftkey/gists{/gist_id}","starred_url":"https://api.github.com/users/shiftkey/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/shiftkey/subscriptions","organizations_url":"https://api.github.com/users/shiftkey/orgs","repos_url":"https://api.github.com/users/shiftkey/repos","events_url":"https://api.github.com/users/shiftkey/events{/privacy}","received_events_url":"https://api.github.com/users/shiftkey/received_events","type":"User","site_admin":true},"created_at":"2015-12-18T04:41:07Z","updated_at":"2015-12-18T04:41:07Z","body":"@Pilchie thanks for the :eyes:\r\n\r\n1. Fair point, and perhaps it's because I'm just dipping my toe in. I initially thought about using something like Fody to rewrite the IL, but I keep not finding that right reason to use Roslyn in anger. We're easing into this whole \"Roslyn\" thing, so I'm going to soldier on anyway for the sake of this thought experiment.\r\n\r\n2. We're not using C# 6 yet in Octokit - that's why we've had these `FormatUri` type methods around. That should make verifying the string contents easier. Let's cross that bridge later.\r\n\r\n3. I thought about that too, but I avoided that because you don't really have a chance to verify it's the right format before runtime (unless I use Fody and make it break the build) - hence me hacking away at Roslyn."}}