{"action":"created","comment":{"url":"https://api.github.com/repos/tengstrand/lein-polylith/pulls/comments/232584561","pull_request_review_id":173811064,"id":232584561,"node_id":"MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDIzMjU4NDU2MQ==","diff_hunk":"@@ -98,8 +98,13 @@\n   (let [levels (+ 2 (count (str/split dir #\"/\")))]\n     (str/join (repeat levels \"../\"))))\n \n+(def env \"The environment as a map, memoized for performance.\"\n+  (memoize #(into {} (System/getenv))))\n+\n (defn sh [& args]\n-  (let [{:keys [exit out err]} (apply shell/sh args)]\n+  (let [;; apply doesn't accept extra args at the end so we must use concat","path":"src/leiningen/polylith/cmd/shared.clj","position":9,"original_position":9,"commit_id":"6edc9a840a4d9fdbd2086319b9434b612cd4869b","original_commit_id":"6edc9a840a4d9fdbd2086319b9434b612cd4869b","user":{"login":"davidrolle","id":5514898,"node_id":"MDQ6VXNlcjU1MTQ4OTg=","avatar_url":"https://avatars2.githubusercontent.com/u/5514898?v=4","gravatar_id":"","url":"https://api.github.com/users/davidrolle","html_url":"https://github.com/davidrolle","followers_url":"https://api.github.com/users/davidrolle/followers","following_url":"https://api.github.com/users/davidrolle/following{/other_user}","gists_url":"https://api.github.com/users/davidrolle/gists{/gist_id}","starred_url":"https://api.github.com/users/davidrolle/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/davidrolle/subscriptions","organizations_url":"https://api.github.com/users/davidrolle/orgs","repos_url":"https://api.github.com/users/davidrolle/repos","events_url":"https://api.github.com/users/davidrolle/events{/privacy}","received_events_url":"https://api.github.com/users/davidrolle/received_events","type":"User","site_admin":false},"body":"I agree that it looks nicer your way, but the reason I did it the other way is that `(into {} (System/getenv))` takes about 1 second to execute. Since `sh` is called quite frequently I thought it's worth saving the env map to speed up the overall execution, and I like the memoization pattern as a simple way to do that (the idea came from [ions-starter](https://github.com/Datomic/ion-starter/blob/cf8f5b68014da2fb2b23a1f394cd00d66d4b6b00/src/datomic/ion/starter.clj#L17).\r\n\r\nBut, I've just some more testing and it seems like `(into {} (System/getenv))` is only slow the first time you run it, after that it's only about 0.1 seconds per run, which is still 10x slower than using memoize but not slow enough to lose any sleep over 🙂\r\n\r\nWhat do you think, keep the memoize or ditch it?","created_at":"2018-11-12T09:30:38Z","updated_at":"2018-11-12T09:30:39Z","html_url":"https://github.com/tengstrand/lein-polylith/pull/18#discussion_r232584561","pull_request_url":"https://api.github.com/repos/tengstrand/lein-polylith/pulls/18","author_association":"NONE","_links":{"self":{"href":"https://api.github.com/repos/tengstrand/lein-polylith/pulls/comments/232584561"},"html":{"href":"https://github.com/tengstrand/lein-polylith/pull/18#discussion_r232584561"},"pull_request":{"href":"https://api.github.com/repos/tengstrand/lein-polylith/pulls/18"}},"in_reply_to_id":232512087},"pull_request":{"url":"https://api.github.com/repos/tengstrand/lein-polylith/pulls/18","id":229854247,"node_id":"MDExOlB1bGxSZXF1ZXN0MjI5ODU0MjQ3","html_url":"https://github.com/tengstrand/lein-polylith/pull/18","diff_url":"https://github.com/tengstrand/lein-polylith/pull/18.diff","patch_url":"https://github.com/tengstrand/lein-polylith/pull/18.patch","issue_url":"https://api.github.com/repos/tengstrand/lein-polylith/issues/18","number":18,"state":"open","locked":false,"title":"Unset lein CLASSPATH in nested sh","user":{"login":"davidrolle","id":5514898,"node_id":"MDQ6VXNlcjU1MTQ4OTg=","avatar_url":"https://avatars2.githubusercontent.com/u/5514898?v=4","gravatar_id":"","url":"https://api.github.com/users/davidrolle","html_url":"https://github.com/davidrolle","followers_url":"https://api.github.com/users/davidrolle/followers","following_url":"https://api.github.com/users/davidrolle/following{/other_user}","gists_url":"https://api.github.com/users/davidrolle/gists{/gist_id}","starred_url":"https://api.github.com/users/davidrolle/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/davidrolle/subscriptions","organizations_url":"https://api.github.com/users/davidrolle/orgs","repos_url":"https://api.github.com/users/davidrolle/repos","events_url":"https://api.github.com/users/davidrolle/events{/privacy}","received_events_url":"https://api.github.com/users/davidrolle/received_events","type":"User","site_admin":false},"body":"This is a fix for #16.\r\n\r\nOn Linux, the `lein` executable is a shell script which runs the lein jar by first setting CLASSPATH. As a precondition, it checks that the CLASSPATH is not already set.\r\n\r\nSince lein-polylith uses the shell to execute more lein commands, there is a problem that the nested shell session gets the env of the parent lein process including the CLASSPATH. This change unsets that CLASSPATH to prevent the nested `lein` call from complaining.\r\n\r\nThis is not a particularly big issue, except that it causes many of the tests to fail on my machine making it difficult for me to develop lein-polylith.\r\n\r\nAs part of this PR I've also fixed the 2 tests which were failing due to formatting errors and/or mistakes in the expected output (I think in one case you've added additional information output but not updated the test).\r\n\r\nFYI - this is my first PR using GitHub so apologies in advance if I've missed some part of the process.","created_at":"2018-11-10T00:21:21Z","updated_at":"2018-11-12T09:30:39Z","closed_at":null,"merged_at":null,"merge_commit_sha":"bb9e097ec602acbd854a378906d3ed26feaa2d24","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[{"id":754198755,"node_id":"MDU6TGFiZWw3NTQxOTg3NTU=","url":"https://api.github.com/repos/tengstrand/lein-polylith/labels/enhancement","name":"enhancement","color":"84b6eb","default":true}],"milestone":null,"commits_url":"https://api.github.com/repos/tengstrand/lein-polylith/pulls/18/commits","review_comments_url":"https://api.github.com/repos/tengstrand/lein-polylith/pulls/18/comments","review_comment_url":"https://api.github.com/repos/tengstrand/lein-polylith/pulls/comments{/number}","comments_url":"https://api.github.com/repos/tengstrand/lein-polylith/issues/18/comments","statuses_url":"https://api.github.com/repos/tengstrand/lein-polylith/statuses/6edc9a840a4d9fdbd2086319b9434b612cd4869b","head":{"label":"nutritionivy:fix/unset-lein-classpath-in-nested-sh","ref":"fix/unset-lein-classpath-in-nested-sh","sha":"6edc9a840a4d9fdbd2086319b9434b612cd4869b","user":{"login":"nutritionivy","id":43878758,"node_id":"MDEyOk9yZ2FuaXphdGlvbjQzODc4NzU4","avatar_url":"https://avatars2.githubusercontent.com/u/43878758?v=4","gravatar_id":"","url":"https://api.github.com/users/nutritionivy","html_url":"https://github.com/nutritionivy","followers_url":"https://api.github.com/users/nutritionivy/followers","following_url":"https://api.github.com/users/nutritionivy/following{/other_user}","gists_url":"https://api.github.com/users/nutritionivy/gists{/gist_id}","starred_url":"https://api.github.com/users/nutritionivy/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nutritionivy/subscriptions","organizations_url":"https://api.github.com/users/nutritionivy/orgs","repos_url":"https://api.github.com/users/nutritionivy/repos","events_url":"https://api.github.com/users/nutritionivy/events{/privacy}","received_events_url":"https://api.github.com/users/nutritionivy/received_events","type":"Organization","site_admin":false},"repo":{"id":156920887,"node_id":"MDEwOlJlcG9zaXRvcnkxNTY5MjA4ODc=","name":"lein-polylith","full_name":"nutritionivy/lein-polylith","private":false,"owner":{"login":"nutritionivy","id":43878758,"node_id":"MDEyOk9yZ2FuaXphdGlvbjQzODc4NzU4","avatar_url":"https://avatars2.githubusercontent.com/u/43878758?v=4","gravatar_id":"","url":"https://api.github.com/users/nutritionivy","html_url":"https://github.com/nutritionivy","followers_url":"https://api.github.com/users/nutritionivy/followers","following_url":"https://api.github.com/users/nutritionivy/following{/other_user}","gists_url":"https://api.github.com/users/nutritionivy/gists{/gist_id}","starred_url":"https://api.github.com/users/nutritionivy/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nutritionivy/subscriptions","organizations_url":"https://api.github.com/users/nutritionivy/orgs","repos_url":"https://api.github.com/users/nutritionivy/repos","events_url":"https://api.github.com/users/nutritionivy/events{/privacy}","received_events_url":"https://api.github.com/users/nutritionivy/received_events","type":"Organization","site_admin":false},"html_url":"https://github.com/nutritionivy/lein-polylith","description":"Libraries and tools for Polylith component based architecture.","fork":true,"url":"https://api.github.com/repos/nutritionivy/lein-polylith","forks_url":"https://api.github.com/repos/nutritionivy/lein-polylith/forks","keys_url":"https://api.github.com/repos/nutritionivy/lein-polylith/keys{/key_id}","collaborators_url":"https://api.github.com/repos/nutritionivy/lein-polylith/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/nutritionivy/lein-polylith/teams","hooks_url":"https://api.github.com/repos/nutritionivy/lein-polylith/hooks","issue_events_url":"https://api.github.com/repos/nutritionivy/lein-polylith/issues/events{/number}","events_url":"https://api.github.com/repos/nutritionivy/lein-polylith/events","assignees_url":"https://api.github.com/repos/nutritionivy/lein-polylith/assignees{/user}","branches_url":"https://api.github.com/repos/nutritionivy/lein-polylith/branches{/branch}","tags_url":"https://api.github.com/repos/nutritionivy/lein-polylith/tags","blobs_url":"https://api.github.com/repos/nutritionivy/lein-polylith/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/nutritionivy/lein-polylith/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/nutritionivy/lein-polylith/git/refs{/sha}","trees_url":"https://api.github.com/repos/nutritionivy/lein-polylith/git/trees{/sha}","statuses_url":"https://api.github.com/repos/nutritionivy/lein-polylith/statuses/{sha}","languages_url":"https://api.github.com/repos/nutritionivy/lein-polylith/languages","stargazers_url":"https://api.github.com/repos/nutritionivy/lein-polylith/stargazers","contributors_url":"https://api.github.com/repos/nutritionivy/lein-polylith/contributors","subscribers_url":"https://api.github.com/repos/nutritionivy/lein-polylith/subscribers","subscription_url":"https://api.github.com/repos/nutritionivy/lein-polylith/subscription","commits_url":"https://api.github.com/repos/nutritionivy/lein-polylith/commits{/sha}","git_commits_url":"https://api.github.com/repos/nutritionivy/lein-polylith/git/commits{/sha}","comments_url":"https://api.github.com/repos/nutritionivy/lein-polylith/comments{/number}","issue_comment_url":"https://api.github.com/repos/nutritionivy/lein-polylith/issues/comments{/number}","contents_url":"https://api.github.com/repos/nutritionivy/lein-polylith/contents/{+path}","compare_url":"https://api.github.com/repos/nutritionivy/lein-polylith/compare/{base}...{head}","merges_url":"https://api.github.com/repos/nutritionivy/lein-polylith/merges","archive_url":"https://api.github.com/repos/nutritionivy/lein-polylith/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/nutritionivy/lein-polylith/downloads","issues_url":"https://api.github.com/repos/nutritionivy/lein-polylith/issues{/number}","pulls_url":"https://api.github.com/repos/nutritionivy/lein-polylith/pulls{/number}","milestones_url":"https://api.github.com/repos/nutritionivy/lein-polylith/milestones{/number}","notifications_url":"https://api.github.com/repos/nutritionivy/lein-polylith/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/nutritionivy/lein-polylith/labels{/name}","releases_url":"https://api.github.com/repos/nutritionivy/lein-polylith/releases{/id}","deployments_url":"https://api.github.com/repos/nutritionivy/lein-polylith/deployments","created_at":"2018-11-09T21:38:17Z","updated_at":"2018-11-09T21:38:19Z","pushed_at":"2018-11-10T11:49:10Z","git_url":"git://github.com/nutritionivy/lein-polylith.git","ssh_url":"git@github.com:nutritionivy/lein-polylith.git","clone_url":"https://github.com/nutritionivy/lein-polylith.git","svn_url":"https://github.com/nutritionivy/lein-polylith","homepage":"","size":5463,"stargazers_count":0,"watchers_count":0,"language":"Clojure","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":0,"license":{"key":"epl-1.0","name":"Eclipse Public License 1.0","spdx_id":"EPL-1.0","url":"https://api.github.com/licenses/epl-1.0","node_id":"MDc6TGljZW5zZTc="},"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"tengstrand:master","ref":"master","sha":"52329e33abfe49a76f4d82d12fb7a2ce0ffe03fb","user":{"login":"tengstrand","id":631272,"node_id":"MDQ6VXNlcjYzMTI3Mg==","avatar_url":"https://avatars2.githubusercontent.com/u/631272?v=4","gravatar_id":"","url":"https://api.github.com/users/tengstrand","html_url":"https://github.com/tengstrand","followers_url":"https://api.github.com/users/tengstrand/followers","following_url":"https://api.github.com/users/tengstrand/following{/other_user}","gists_url":"https://api.github.com/users/tengstrand/gists{/gist_id}","starred_url":"https://api.github.com/users/tengstrand/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tengstrand/subscriptions","organizations_url":"https://api.github.com/users/tengstrand/orgs","repos_url":"https://api.github.com/users/tengstrand/repos","events_url":"https://api.github.com/users/tengstrand/events{/privacy}","received_events_url":"https://api.github.com/users/tengstrand/received_events","type":"User","site_admin":false},"repo":{"id":111202291,"node_id":"MDEwOlJlcG9zaXRvcnkxMTEyMDIyOTE=","name":"lein-polylith","full_name":"tengstrand/lein-polylith","private":false,"owner":{"login":"tengstrand","id":631272,"node_id":"MDQ6VXNlcjYzMTI3Mg==","avatar_url":"https://avatars2.githubusercontent.com/u/631272?v=4","gravatar_id":"","url":"https://api.github.com/users/tengstrand","html_url":"https://github.com/tengstrand","followers_url":"https://api.github.com/users/tengstrand/followers","following_url":"https://api.github.com/users/tengstrand/following{/other_user}","gists_url":"https://api.github.com/users/tengstrand/gists{/gist_id}","starred_url":"https://api.github.com/users/tengstrand/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tengstrand/subscriptions","organizations_url":"https://api.github.com/users/tengstrand/orgs","repos_url":"https://api.github.com/users/tengstrand/repos","events_url":"https://api.github.com/users/tengstrand/events{/privacy}","received_events_url":"https://api.github.com/users/tengstrand/received_events","type":"User","site_admin":false},"html_url":"https://github.com/tengstrand/lein-polylith","description":"Libraries and tools for Polylith component based architecture.","fork":false,"url":"https://api.github.com/repos/tengstrand/lein-polylith","forks_url":"https://api.github.com/repos/tengstrand/lein-polylith/forks","keys_url":"https://api.github.com/repos/tengstrand/lein-polylith/keys{/key_id}","collaborators_url":"https://api.github.com/repos/tengstrand/lein-polylith/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/tengstrand/lein-polylith/teams","hooks_url":"https://api.github.com/repos/tengstrand/lein-polylith/hooks","issue_events_url":"https://api.github.com/repos/tengstrand/lein-polylith/issues/events{/number}","events_url":"https://api.github.com/repos/tengstrand/lein-polylith/events","assignees_url":"https://api.github.com/repos/tengstrand/lein-polylith/assignees{/user}","branches_url":"https://api.github.com/repos/tengstrand/lein-polylith/branches{/branch}","tags_url":"https://api.github.com/repos/tengstrand/lein-polylith/tags","blobs_url":"https://api.github.com/repos/tengstrand/lein-polylith/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/tengstrand/lein-polylith/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/tengstrand/lein-polylith/git/refs{/sha}","trees_url":"https://api.github.com/repos/tengstrand/lein-polylith/git/trees{/sha}","statuses_url":"https://api.github.com/repos/tengstrand/lein-polylith/statuses/{sha}","languages_url":"https://api.github.com/repos/tengstrand/lein-polylith/languages","stargazers_url":"https://api.github.com/repos/tengstrand/lein-polylith/stargazers","contributors_url":"https://api.github.com/repos/tengstrand/lein-polylith/contributors","subscribers_url":"https://api.github.com/repos/tengstrand/lein-polylith/subscribers","subscription_url":"https://api.github.com/repos/tengstrand/lein-polylith/subscription","commits_url":"https://api.github.com/repos/tengstrand/lein-polylith/commits{/sha}","git_commits_url":"https://api.github.com/repos/tengstrand/lein-polylith/git/commits{/sha}","comments_url":"https://api.github.com/repos/tengstrand/lein-polylith/comments{/number}","issue_comment_url":"https://api.github.com/repos/tengstrand/lein-polylith/issues/comments{/number}","contents_url":"https://api.github.com/repos/tengstrand/lein-polylith/contents/{+path}","compare_url":"https://api.github.com/repos/tengstrand/lein-polylith/compare/{base}...{head}","merges_url":"https://api.github.com/repos/tengstrand/lein-polylith/merges","archive_url":"https://api.github.com/repos/tengstrand/lein-polylith/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/tengstrand/lein-polylith/downloads","issues_url":"https://api.github.com/repos/tengstrand/lein-polylith/issues{/number}","pulls_url":"https://api.github.com/repos/tengstrand/lein-polylith/pulls{/number}","milestones_url":"https://api.github.com/repos/tengstrand/lein-polylith/milestones{/number}","notifications_url":"https://api.github.com/repos/tengstrand/lein-polylith/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/tengstrand/lein-polylith/labels{/name}","releases_url":"https://api.github.com/repos/tengstrand/lein-polylith/releases{/id}","deployments_url":"https://api.github.com/repos/tengstrand/lein-polylith/deployments","created_at":"2017-11-18T12:05:13Z","updated_at":"2018-11-09T21:37:57Z","pushed_at":"2018-11-10T11:49:12Z","git_url":"git://github.com/tengstrand/lein-polylith.git","ssh_url":"git@github.com:tengstrand/lein-polylith.git","clone_url":"https://github.com/tengstrand/lein-polylith.git","svn_url":"https://github.com/tengstrand/lein-polylith","homepage":"","size":5436,"stargazers_count":58,"watchers_count":58,"language":"Clojure","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":4,"mirror_url":null,"archived":false,"open_issues_count":4,"license":{"key":"epl-1.0","name":"Eclipse Public License 1.0","spdx_id":"EPL-1.0","url":"https://api.github.com/licenses/epl-1.0","node_id":"MDc6TGljZW5zZTc="},"forks":4,"open_issues":4,"watchers":58,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/tengstrand/lein-polylith/pulls/18"},"html":{"href":"https://github.com/tengstrand/lein-polylith/pull/18"},"issue":{"href":"https://api.github.com/repos/tengstrand/lein-polylith/issues/18"},"comments":{"href":"https://api.github.com/repos/tengstrand/lein-polylith/issues/18/comments"},"review_comments":{"href":"https://api.github.com/repos/tengstrand/lein-polylith/pulls/18/comments"},"review_comment":{"href":"https://api.github.com/repos/tengstrand/lein-polylith/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/tengstrand/lein-polylith/pulls/18/commits"},"statuses":{"href":"https://api.github.com/repos/tengstrand/lein-polylith/statuses/6edc9a840a4d9fdbd2086319b9434b612cd4869b"}},"author_association":"NONE"}}
{"actor":{"display_login":"davidrolle"}}