GitHub PullRequestReviewCommentEvent at 2014-01-13 22:54:59
Repository: jmake
Actor: countmdm
Payload: {"comment":{"url":"https://api.github.com/repos/pantsbuild/jmake/pulls/comments/8844303","id":8844303,"diff_hunk":"@@ -324,13 +329,18 @@ public void mainProgrammatic(String args[]) throws\n             }\n \n             Utils.startTiming(Utils.TIMING_PDBREAD);\n-            PCDContainer pcdc = PCDContainer.load(pdbFileName);\n+            PCDContainer pcdc;\n+            if (pdbTextFormat) {\n+                pcdc = PCDContainer.loadFromText(pdbFileName);\n+            } else {\n+                pcdc = PCDContainer.load(pdbFileName);\n+            }\n             Utils.stopAndPrintTiming(\"DB read\", Utils.TIMING_PDBREAD);\n \n             pcdm = new PCDManager(pcdc, allProjectJavaFileNames,\n-                    addedJavaFileNames, removedJavaFileNames, updatedJavaFileNames,\n-                    destDir, javacAddArgs,\n-                    failOnDependentJar, noWarnOnDependentJar);\n+                addedJavaFileNames, removedJavaFileNames, updatedJavaFileNames,\n+                destDir, javacAddArgs, failOnDependentJar, noWarnOnDependentJar);\n+            pcdm.setPdbTextFormat(pdbTextFormat);","path":"src/com/sun/tools/jmake/Main.java","position":51,"original_position":51,"commit_id":"609a9b526e4a4ac6c9f14c44201acc2a2b1e1585","original_commit_id":"609a9b526e4a4ac6c9f14c44201acc2a2b1e1585","user":{"login":"countmdm","id":6394099,"avatar_url":"https://gravatar.com/avatar/a3e7d5e62bac551124ed1be7f0a04b1c?d=https%3A%2F%2Fidenticons.github.com%2F86256a95dc7498b5414a01c00f07af70.png&r=x","gravatar_id":"a3e7d5e62bac551124ed1be7f0a04b1c","url":"https://api.github.com/users/countmdm","html_url":"https://github.com/countmdm","followers_url":"https://api.github.com/users/countmdm/followers","following_url":"https://api.github.com/users/countmdm/following{/other_user}","gists_url":"https://api.github.com/users/countmdm/gists{/gist_id}","starred_url":"https://api.github.com/users/countmdm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/countmdm/subscriptions","organizations_url":"https://api.github.com/users/countmdm/orgs","repos_url":"https://api.github.com/users/countmdm/repos","events_url":"https://api.github.com/users/countmdm/events{/privacy}","received_events_url":"https://api.github.com/users/countmdm/received_events","type":"User","site_admin":false},"body":"Setting DB format via a separate method seems awkward/error-prone given that this format is already stored in PCDContainer instance, which is passed to this constructor. So I suggest to remove this method and add PCDContainer.isDbInTextFormat() or some such instead.","created_at":"2014-01-13T22:54:59Z","updated_at":"2014-01-13T22:54:59Z","html_url":"https://github.com/pantsbuild/jmake/pull/2#discussion_r8844303","pull_request_url":"https://api.github.com/repos/pantsbuild/jmake/pulls/2","_links":{"self":{"href":"https://api.github.com/repos/pantsbuild/jmake/pulls/comments/8844303"},"html":{"href":"https://github.com/pantsbuild/jmake/pull/2#discussion_r8844303"},"pull_request":{"href":"https://api.github.com/repos/pantsbuild/jmake/pulls/2"}}}}