{"action":"created","issue":{"url":"https://api.github.com/repos/hawkular/hawkular-metrics/issues/191","labels_url":"https://api.github.com/repos/hawkular/hawkular-metrics/issues/191/labels{/name}","comments_url":"https://api.github.com/repos/hawkular/hawkular-metrics/issues/191/comments","events_url":"https://api.github.com/repos/hawkular/hawkular-metrics/issues/191/events","html_url":"https://github.com/hawkular/hawkular-metrics/pull/191","id":65309658,"number":191,"title":"REST service generator based on annotation processing","user":{"login":"tsegismont","id":1500598,"avatar_url":"https://avatars.githubusercontent.com/u/1500598?v=3","gravatar_id":"","url":"https://api.github.com/users/tsegismont","html_url":"https://github.com/tsegismont","followers_url":"https://api.github.com/users/tsegismont/followers","following_url":"https://api.github.com/users/tsegismont/following{/other_user}","gists_url":"https://api.github.com/users/tsegismont/gists{/gist_id}","starred_url":"https://api.github.com/users/tsegismont/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tsegismont/subscriptions","organizations_url":"https://api.github.com/users/tsegismont/orgs","repos_url":"https://api.github.com/users/tsegismont/repos","events_url":"https://api.github.com/users/tsegismont/events{/privacy}","received_events_url":"https://api.github.com/users/tsegismont/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":5,"created_at":"2015-03-30T21:49:05Z","updated_at":"2015-03-31T15:48:28Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/hawkular/hawkular-metrics/pulls/191","html_url":"https://github.com/hawkular/hawkular-metrics/pull/191","diff_url":"https://github.com/hawkular/hawkular-metrics/pull/191.diff","patch_url":"https://github.com/hawkular/hawkular-metrics/pull/191.patch"},"body":"**Not ready yet**\r\n\r\nThe generator bridges the gap between the JAX-RS AsyncResponse model and the C* driver ListenableFuture model.\r\n\r\nIt frees us from copy/pasting #executeAsync calls and avoid writing possibly long, nested lambdas (not super easy to read, not debugger-friendly, not checkstyle-friendly).\r\n\r\nIt's a first shot so it still needs some polishing (no Javadoc, more code to factor out).\r\n\r\nIt should work fine with the docgen profile, but needs to be checked."},"comment":{"url":"https://api.github.com/repos/hawkular/hawkular-metrics/issues/comments/88139445","html_url":"https://github.com/hawkular/hawkular-metrics/pull/191#issuecomment-88139445","issue_url":"https://api.github.com/repos/hawkular/hawkular-metrics/issues/191","id":88139445,"user":{"login":"tsegismont","id":1500598,"avatar_url":"https://avatars.githubusercontent.com/u/1500598?v=3","gravatar_id":"","url":"https://api.github.com/users/tsegismont","html_url":"https://github.com/tsegismont","followers_url":"https://api.github.com/users/tsegismont/followers","following_url":"https://api.github.com/users/tsegismont/following{/other_user}","gists_url":"https://api.github.com/users/tsegismont/gists{/gist_id}","starred_url":"https://api.github.com/users/tsegismont/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tsegismont/subscriptions","organizations_url":"https://api.github.com/users/tsegismont/orgs","repos_url":"https://api.github.com/users/tsegismont/repos","events_url":"https://api.github.com/users/tsegismont/events{/privacy}","received_events_url":"https://api.github.com/users/tsegismont/received_events","type":"User","site_admin":false},"created_at":"2015-03-31T15:48:28Z","updated_at":"2015-03-31T15:48:28Z","body":"I was writing my previous comment while you sent yours. My answers inline.\n\nLe 31/03/2015 17:24, jsanda a écrit :\n> I have some reservations about this PR. First, we are not\n> copying/pasting calls of executeAsync. We call executeAsync in several\n> method using different arguments. That is code reuse, not copy/paste.\n\nYou're right. I was referring to the way I'm working with it: copy past \nthe block from another method, then edit the lamdba.\n\n>\n> The long, nested lambdas to which you refer are simply the same method\n> bodies prior to introducing the executeAsync method and are not wrapped\n> in a function. The caller of that function, executeAsync, handles the\n> futures callback code, thus eliminating what was previously some boiler\n> plate code.\n\nNot exactly. The #executeAsync construct has changed the way the service \nmethod is implemented: the lambda passed as argument must return a \nListenable<Future>. Before, service method had to deal with FutureCallbacks\n\nNote that I started this PR because I think #executeAsync was a good \nstep forward. The difference with the annotation processor is that the \nlambda passed as argument to #executeAsync is now the actual handler method.\n\n>\n> I also disagree that the code is more difficult to read. I am not saying\n> it is easier, but I do not think that wrapping those method bodies in\n> lambda functions make it less readable. I suppose it is a matter of\n> taste/preference.\n>\n\nThe nested lambda blocks are at least 8 column indented. Given that we \nwork with ListenableFuture<List<AvaiabilityMetric> instances, it leaves \njust a few column for actual code.\n\n> Can you elaborate on the debugger-friendly aspect? I have no problem\n> setting breakpoints in or stepping through a lambda function.\n\nDebugger-friendly was a mistake, I meant stack trace friendly. When your \nlambda is 1,2,3 lines it's fine, it does a few things. Having lambdas of \n30 lines is a mistake IMO.\n\n>\n> It is not checkstyle-friendly because it is new. I would rather put it\n> that checkstyle is not lamba-friendly :)\n>\n\nI meant that the service handler code with #executeAsync forces you to \nloose a bunch of columns before you reach the margin.\n\n\n> Lastly, I am -1 on code generation.\n\nWhy? Just on principles?\n\nGenerating boiler plate code with an annotation processor is very useful.\n"}}