{"action":"created","comment":{"url":"https://api.github.com/repos/Homebrew/brew/pulls/comments/142349999","pull_request_review_id":66696371,"id":142349999,"diff_hunk":"@@ -26,13 +26,14 @@ def summarize\n         path.relative_path_from(cask.staged_path).to_s\n       end\n \n-      def install_phase(**options)\n-        run_installer(**options)\n+      def install_phase(**install_options)\n+        run_installer(**install_options)\n       end\n \n       private\n \n-      def run_installer(command: nil, verbose: false, **options)\n+      def run_installer(command: nil, verbose: false,\n+        **_ignored_install_options)","path":"Library/Homebrew/cask/lib/hbc/artifact/pkg.rb","position":null,"original_position":14,"commit_id":"fbd526ffe6a619d538193fbc7eb93b6e74c44b00","original_commit_id":"b4a528d3449dff1dbce127da082b1373cb765b81","user":{"login":"claui","id":1239874,"avatar_url":"https://avatars3.githubusercontent.com/u/1239874?v=4","gravatar_id":"","url":"https://api.github.com/users/claui","html_url":"https://github.com/claui","followers_url":"https://api.github.com/users/claui/followers","following_url":"https://api.github.com/users/claui/following{/other_user}","gists_url":"https://api.github.com/users/claui/gists{/gist_id}","starred_url":"https://api.github.com/users/claui/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/claui/subscriptions","organizations_url":"https://api.github.com/users/claui/orgs","repos_url":"https://api.github.com/users/claui/repos","events_url":"https://api.github.com/users/claui/events{/privacy}","received_events_url":"https://api.github.com/users/claui/received_events","type":"User","site_admin":false},"body":"> I wouldn't say that `options.fetch(:force, false)` is the obvious fix\r\n\r\nI feel like there’s a misunderstanding here. Maybe my wording has not been clear enough. Let me put it in other words.\r\n\r\nThis is what the code **says today:**\r\n\r\n```\r\n[…] stanza_options.fetch(:allow_untrusted, false) […]\r\n```\r\n\r\nBecause **all we have** is `stanza_options `, and `:allow_untrusted` **works fine** because it’s in `stanza_options`, it’s a very **natural mistake** to assume that `:force` would be in `stanza_options` just as well:\r\n\r\n```\r\n[…] stanza_options.fetch(:allow_untrusted, false) […]\r\n[…]\r\n[…] stanza_options.fetch(:force, false) […]\r\n```\r\n\r\nNote: All I’m saying is that `stanza_options.fetch(:force, false)` is a _natural mistake_ to make while adding the feature. I’m not saying it’s an obvious fix. There’s not even an issue yet that would need fixing.\r\n\r\nAs `stanza_options.fetch(:force, false)` will of course **not work as expected**, a PR author might then be inclined to rename `_` to `stanza_options` because it seems to make perfect sense at this moment.\r\n\r\nRenaming `_` to `stanza_options` would **seemingly fix** the issue; however, not only would such an _obvious fix_ constitute a second mistake, it would also subtly break `allow_untrusted` once again, just like it’s broken today.\r\n\r\n<br/>\r\n\r\n> I still think **_ is better\r\n\r\nWhy do you think it’s better? I’d really like to understand.\r\nWe’re dealing with two similar but different options hashes. It’s really easy to mix them up. I feel it’s important to give a hint for future developers here.\r\n\r\n<br/>\r\n\r\n> because **_options won't become **options, but rather an additional named parameter should be added.\r\n\r\nHow is this an argument in favor of `**_`?\r\n\r\nPlease keep in mind that this issue currently breaks a dozen of casks. That’s no big deal in itself but all I’m trying to do here is to fix it **and to prevent this particular mistake** from happening again.\r\n","created_at":"2017-10-03T09:13:40Z","updated_at":"2017-10-03T09:13:40Z","html_url":"https://github.com/Homebrew/brew/pull/3256#discussion_r142349999","pull_request_url":"https://api.github.com/repos/Homebrew/brew/pulls/3256","author_association":"NONE","_links":{"self":{"href":"https://api.github.com/repos/Homebrew/brew/pulls/comments/142349999"},"html":{"href":"https://github.com/Homebrew/brew/pull/3256#discussion_r142349999"},"pull_request":{"href":"https://api.github.com/repos/Homebrew/brew/pulls/3256"}},"in_reply_to_id":142075632},"pull_request":{"url":"https://api.github.com/repos/Homebrew/brew/pulls/3256","id":144092713,"html_url":"https://github.com/Homebrew/brew/pull/3256","diff_url":"https://github.com/Homebrew/brew/pull/3256.diff","patch_url":"https://github.com/Homebrew/brew/pull/3256.patch","issue_url":"https://api.github.com/repos/Homebrew/brew/issues/3256","number":3256,"state":"open","locked":false,"title":"Unshadow `allow_untrusted` option for pkg artifact","user":{"login":"claui","id":1239874,"avatar_url":"https://avatars3.githubusercontent.com/u/1239874?v=4","gravatar_id":"","url":"https://api.github.com/users/claui","html_url":"https://github.com/claui","followers_url":"https://api.github.com/users/claui/followers","following_url":"https://api.github.com/users/claui/following{/other_user}","gists_url":"https://api.github.com/users/claui/gists{/gist_id}","starred_url":"https://api.github.com/users/claui/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/claui/subscriptions","organizations_url":"https://api.github.com/users/claui/orgs","repos_url":"https://api.github.com/users/claui/repos","events_url":"https://api.github.com/users/claui/events{/privacy}","received_events_url":"https://api.github.com/users/claui/received_events","type":"User","site_admin":false},"body":"The `pkg` stanza has an option called `allow_untrusted`, which is supposed to cause `/usr/sbin/installer` to be called with the `-allowUntrusted` switch.\r\n\r\nPR #3141 seems to have renamed the `pkg_install_opts` field to `options`. At the same time, it introduces an `options` parameter for the `run_installer` method, which shadows the `options` getter method, causing the `allow_untrusted` option to be silently ignored.\r\n\r\nThe issue affects just the `pkg` stanza because `Hbc::Artifact::Pkg` is the only artifact class that has an `options` method.\r\n\r\nThis PR removes the shadowing ~~by renaming the method parameters~~ by renaming the field to `stanza_options`; in one case, it uses ~~`ignored_install_options`~~ `_options` for a parameter name (instead of the more canonical `_`) for the sake of clarity.\r\n\r\n**Edit:** last paragraph as per @reitermarkus’s suggestions.","created_at":"2017-10-01T22:59:25Z","updated_at":"2017-10-03T09:13:40Z","closed_at":null,"merged_at":null,"merge_commit_sha":"c00c2651816d088007510e3ca4ee2e94539c7eab","assignee":null,"assignees":[],"requested_reviewers":[],"milestone":null,"commits_url":"https://api.github.com/repos/Homebrew/brew/pulls/3256/commits","review_comments_url":"https://api.github.com/repos/Homebrew/brew/pulls/3256/comments","review_comment_url":"https://api.github.com/repos/Homebrew/brew/pulls/comments{/number}","comments_url":"https://api.github.com/repos/Homebrew/brew/issues/3256/comments","statuses_url":"https://api.github.com/repos/Homebrew/brew/statuses/fbd526ffe6a619d538193fbc7eb93b6e74c44b00","head":{"label":"claui:pkg-unshadow-options","ref":"pkg-unshadow-options","sha":"fbd526ffe6a619d538193fbc7eb93b6e74c44b00","user":{"login":"claui","id":1239874,"avatar_url":"https://avatars3.githubusercontent.com/u/1239874?v=4","gravatar_id":"","url":"https://api.github.com/users/claui","html_url":"https://github.com/claui","followers_url":"https://api.github.com/users/claui/followers","following_url":"https://api.github.com/users/claui/following{/other_user}","gists_url":"https://api.github.com/users/claui/gists{/gist_id}","starred_url":"https://api.github.com/users/claui/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/claui/subscriptions","organizations_url":"https://api.github.com/users/claui/orgs","repos_url":"https://api.github.com/users/claui/repos","events_url":"https://api.github.com/users/claui/events{/privacy}","received_events_url":"https://api.github.com/users/claui/received_events","type":"User","site_admin":false},"repo":{"id":76072820,"name":"brew","full_name":"claui/brew","owner":{"login":"claui","id":1239874,"avatar_url":"https://avatars3.githubusercontent.com/u/1239874?v=4","gravatar_id":"","url":"https://api.github.com/users/claui","html_url":"https://github.com/claui","followers_url":"https://api.github.com/users/claui/followers","following_url":"https://api.github.com/users/claui/following{/other_user}","gists_url":"https://api.github.com/users/claui/gists{/gist_id}","starred_url":"https://api.github.com/users/claui/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/claui/subscriptions","organizations_url":"https://api.github.com/users/claui/orgs","repos_url":"https://api.github.com/users/claui/repos","events_url":"https://api.github.com/users/claui/events{/privacy}","received_events_url":"https://api.github.com/users/claui/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/claui/brew","description":":beer: The missing package manager for macOS","fork":true,"url":"https://api.github.com/repos/claui/brew","forks_url":"https://api.github.com/repos/claui/brew/forks","keys_url":"https://api.github.com/repos/claui/brew/keys{/key_id}","collaborators_url":"https://api.github.com/repos/claui/brew/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/claui/brew/teams","hooks_url":"https://api.github.com/repos/claui/brew/hooks","issue_events_url":"https://api.github.com/repos/claui/brew/issues/events{/number}","events_url":"https://api.github.com/repos/claui/brew/events","assignees_url":"https://api.github.com/repos/claui/brew/assignees{/user}","branches_url":"https://api.github.com/repos/claui/brew/branches{/branch}","tags_url":"https://api.github.com/repos/claui/brew/tags","blobs_url":"https://api.github.com/repos/claui/brew/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/claui/brew/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/claui/brew/git/refs{/sha}","trees_url":"https://api.github.com/repos/claui/brew/git/trees{/sha}","statuses_url":"https://api.github.com/repos/claui/brew/statuses/{sha}","languages_url":"https://api.github.com/repos/claui/brew/languages","stargazers_url":"https://api.github.com/repos/claui/brew/stargazers","contributors_url":"https://api.github.com/repos/claui/brew/contributors","subscribers_url":"https://api.github.com/repos/claui/brew/subscribers","subscription_url":"https://api.github.com/repos/claui/brew/subscription","commits_url":"https://api.github.com/repos/claui/brew/commits{/sha}","git_commits_url":"https://api.github.com/repos/claui/brew/git/commits{/sha}","comments_url":"https://api.github.com/repos/claui/brew/comments{/number}","issue_comment_url":"https://api.github.com/repos/claui/brew/issues/comments{/number}","contents_url":"https://api.github.com/repos/claui/brew/contents/{+path}","compare_url":"https://api.github.com/repos/claui/brew/compare/{base}...{head}","merges_url":"https://api.github.com/repos/claui/brew/merges","archive_url":"https://api.github.com/repos/claui/brew/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/claui/brew/downloads","issues_url":"https://api.github.com/repos/claui/brew/issues{/number}","pulls_url":"https://api.github.com/repos/claui/brew/pulls{/number}","milestones_url":"https://api.github.com/repos/claui/brew/milestones{/number}","notifications_url":"https://api.github.com/repos/claui/brew/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/claui/brew/labels{/name}","releases_url":"https://api.github.com/repos/claui/brew/releases{/id}","deployments_url":"https://api.github.com/repos/claui/brew/deployments","created_at":"2016-12-09T21:49:07Z","updated_at":"2016-12-09T21:49:09Z","pushed_at":"2017-10-02T11:35:08Z","git_url":"git://github.com/claui/brew.git","ssh_url":"git@github.com:claui/brew.git","clone_url":"https://github.com/claui/brew.git","svn_url":"https://github.com/claui/brew","homepage":"http://brew.sh","size":21700,"stargazers_count":0,"watchers_count":0,"language":"Ruby","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"Homebrew:master","ref":"master","sha":"e7cf509cfb5891ad32eebf8d2ec58bd936af6694","user":{"login":"Homebrew","id":1503512,"avatar_url":"https://avatars2.githubusercontent.com/u/1503512?v=4","gravatar_id":"","url":"https://api.github.com/users/Homebrew","html_url":"https://github.com/Homebrew","followers_url":"https://api.github.com/users/Homebrew/followers","following_url":"https://api.github.com/users/Homebrew/following{/other_user}","gists_url":"https://api.github.com/users/Homebrew/gists{/gist_id}","starred_url":"https://api.github.com/users/Homebrew/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Homebrew/subscriptions","organizations_url":"https://api.github.com/users/Homebrew/orgs","repos_url":"https://api.github.com/users/Homebrew/repos","events_url":"https://api.github.com/users/Homebrew/events{/privacy}","received_events_url":"https://api.github.com/users/Homebrew/received_events","type":"Organization","site_admin":false},"repo":{"id":53238813,"name":"brew","full_name":"Homebrew/brew","owner":{"login":"Homebrew","id":1503512,"avatar_url":"https://avatars2.githubusercontent.com/u/1503512?v=4","gravatar_id":"","url":"https://api.github.com/users/Homebrew","html_url":"https://github.com/Homebrew","followers_url":"https://api.github.com/users/Homebrew/followers","following_url":"https://api.github.com/users/Homebrew/following{/other_user}","gists_url":"https://api.github.com/users/Homebrew/gists{/gist_id}","starred_url":"https://api.github.com/users/Homebrew/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Homebrew/subscriptions","organizations_url":"https://api.github.com/users/Homebrew/orgs","repos_url":"https://api.github.com/users/Homebrew/repos","events_url":"https://api.github.com/users/Homebrew/events{/privacy}","received_events_url":"https://api.github.com/users/Homebrew/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/Homebrew/brew","description":"🍺 The missing package manager for macOS","fork":false,"url":"https://api.github.com/repos/Homebrew/brew","forks_url":"https://api.github.com/repos/Homebrew/brew/forks","keys_url":"https://api.github.com/repos/Homebrew/brew/keys{/key_id}","collaborators_url":"https://api.github.com/repos/Homebrew/brew/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/Homebrew/brew/teams","hooks_url":"https://api.github.com/repos/Homebrew/brew/hooks","issue_events_url":"https://api.github.com/repos/Homebrew/brew/issues/events{/number}","events_url":"https://api.github.com/repos/Homebrew/brew/events","assignees_url":"https://api.github.com/repos/Homebrew/brew/assignees{/user}","branches_url":"https://api.github.com/repos/Homebrew/brew/branches{/branch}","tags_url":"https://api.github.com/repos/Homebrew/brew/tags","blobs_url":"https://api.github.com/repos/Homebrew/brew/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/Homebrew/brew/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/Homebrew/brew/git/refs{/sha}","trees_url":"https://api.github.com/repos/Homebrew/brew/git/trees{/sha}","statuses_url":"https://api.github.com/repos/Homebrew/brew/statuses/{sha}","languages_url":"https://api.github.com/repos/Homebrew/brew/languages","stargazers_url":"https://api.github.com/repos/Homebrew/brew/stargazers","contributors_url":"https://api.github.com/repos/Homebrew/brew/contributors","subscribers_url":"https://api.github.com/repos/Homebrew/brew/subscribers","subscription_url":"https://api.github.com/repos/Homebrew/brew/subscription","commits_url":"https://api.github.com/repos/Homebrew/brew/commits{/sha}","git_commits_url":"https://api.github.com/repos/Homebrew/brew/git/commits{/sha}","comments_url":"https://api.github.com/repos/Homebrew/brew/comments{/number}","issue_comment_url":"https://api.github.com/repos/Homebrew/brew/issues/comments{/number}","contents_url":"https://api.github.com/repos/Homebrew/brew/contents/{+path}","compare_url":"https://api.github.com/repos/Homebrew/brew/compare/{base}...{head}","merges_url":"https://api.github.com/repos/Homebrew/brew/merges","archive_url":"https://api.github.com/repos/Homebrew/brew/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/Homebrew/brew/downloads","issues_url":"https://api.github.com/repos/Homebrew/brew/issues{/number}","pulls_url":"https://api.github.com/repos/Homebrew/brew/pulls{/number}","milestones_url":"https://api.github.com/repos/Homebrew/brew/milestones{/number}","notifications_url":"https://api.github.com/repos/Homebrew/brew/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/Homebrew/brew/labels{/name}","releases_url":"https://api.github.com/repos/Homebrew/brew/releases{/id}","deployments_url":"https://api.github.com/repos/Homebrew/brew/deployments","created_at":"2016-03-06T05:08:38Z","updated_at":"2017-10-03T08:01:10Z","pushed_at":"2017-10-03T08:56:49Z","git_url":"git://github.com/Homebrew/brew.git","ssh_url":"git@github.com:Homebrew/brew.git","clone_url":"https://github.com/Homebrew/brew.git","svn_url":"https://github.com/Homebrew/brew","homepage":"https://brew.sh","size":21699,"stargazers_count":8970,"watchers_count":8970,"language":"Ruby","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":true,"forks_count":2267,"mirror_url":null,"open_issues_count":36,"forks":2267,"open_issues":36,"watchers":8970,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/Homebrew/brew/pulls/3256"},"html":{"href":"https://github.com/Homebrew/brew/pull/3256"},"issue":{"href":"https://api.github.com/repos/Homebrew/brew/issues/3256"},"comments":{"href":"https://api.github.com/repos/Homebrew/brew/issues/3256/comments"},"review_comments":{"href":"https://api.github.com/repos/Homebrew/brew/pulls/3256/comments"},"review_comment":{"href":"https://api.github.com/repos/Homebrew/brew/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/Homebrew/brew/pulls/3256/commits"},"statuses":{"href":"https://api.github.com/repos/Homebrew/brew/statuses/fbd526ffe6a619d538193fbc7eb93b6e74c44b00"}},"author_association":"NONE"}}
{"actor":{"display_login":"claui"}}