GitHub PullRequestReviewCommentEvent at 2014-02-11 04:54:54
Repository: framework
Actor: Shadowfiend
Payload: {"comment":{"url":"https://api.github.com/repos/lift/framework/pulls/comments/9613688","id":9613688,"diff_hunk":"@@ -2600,7 +2600,17 @@ trait S extends HasParams with Loggable with UserAgentCalculator {\n       f\n     }\n \n-  def formFuncName: String = if (Props.testMode && !disableTestFuncNames_?) {\n+  def formFuncName: String = LiftRules.funcNameGenerator()\n+\n+  /** Default func-name logic during test-mode. */\n+  def generateTestFuncName: String =\n+    if (disableTestFuncNames_?)\n+      generateFuncName\n+    else\n+      generatePredictableFuncName\n+\n+  /** Generates a func-name based on the location in the call-site source code. */\n+  def generatePredictableFuncName: String = {","path":"web/webkit/src/main/scala/net/liftweb/http/S.scala","position":15,"original_position":15,"commit_id":"adddad4636ad6c724dd8094462599dbbb2afad0f","original_commit_id":"adddad4636ad6c724dd8094462599dbbb2afad0f","user":{"login":"Shadowfiend","id":8245,"avatar_url":"https://gravatar.com/avatar/36b4b730bc6d058fd0737fc44119ed6f?d=https%3A%2F%2Fidenticons.github.com%2Fc9e5c2b59d98488fe1070e744041ea0e.png&r=x","gravatar_id":"36b4b730bc6d058fd0737fc44119ed6f","url":"https://api.github.com/users/Shadowfiend","html_url":"https://github.com/Shadowfiend","followers_url":"https://api.github.com/users/Shadowfiend/followers","following_url":"https://api.github.com/users/Shadowfiend/following{/other_user}","gists_url":"https://api.github.com/users/Shadowfiend/gists{/gist_id}","starred_url":"https://api.github.com/users/Shadowfiend/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Shadowfiend/subscriptions","organizations_url":"https://api.github.com/users/Shadowfiend/orgs","repos_url":"https://api.github.com/users/Shadowfiend/repos","events_url":"https://api.github.com/users/Shadowfiend/events{/privacy}","received_events_url":"https://api.github.com/users/Shadowfiend/received_events","type":"User","site_admin":false},"body":"I meant runtime-replaceable. I'm also not sure it should be publicly callable.\r\n\r\nIn general, I have a preference for encoding some things in the function name that others put in the comment above it. I find that makes it more likely for the thing that describes what the function does to be updated when the implementation changes, vs the comment which is often left behind.\r\n\r\nNet net, I'm good to go on this. Haven't seen any other objections, so I'm going to go ahead and merge it. Hang onto your butts :sunglasses: ","created_at":"2014-02-11T04:54:54Z","updated_at":"2014-02-11T04:54:54Z","html_url":"https://github.com/lift/framework/pull/1506#discussion_r9613688","pull_request_url":"https://api.github.com/repos/lift/framework/pulls/1506","_links":{"self":{"href":"https://api.github.com/repos/lift/framework/pulls/comments/9613688"},"html":{"href":"https://github.com/lift/framework/pull/1506#discussion_r9613688"},"pull_request":{"href":"https://api.github.com/repos/lift/framework/pulls/1506"}}}}