The Wayback Machine - https://web.archive.org/web/20210902175746/https://github.com/cli/cli/pull/3992
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add browser option to config #3992

Merged
merged 4 commits into from Aug 17, 2021
Merged

add browser option to config #3992

merged 4 commits into from Aug 17, 2021

Conversation

@despreston
Copy link
Contributor

@despreston despreston commented Jul 13, 2021

Allows setting the path to the browser using the config.

Closes #858

@cliAutomation cliAutomation added this to Needs review 🤔 in The GitHub CLI Jul 13, 2021
Allows setting the path to the browser using the config.

Closes #858
@despreston despreston force-pushed the despreston:858-config-browser branch from 79fe649 to c95f30a Jul 13, 2021
@mislav mislav assigned vilmibm and samcoe and unassigned vilmibm Aug 4, 2021
The GitHub CLI automation moved this from Needs review 🤔 to Needs to be merged 🎉 Aug 17, 2021
@samcoe
samcoe approved these changes Aug 17, 2021
Copy link
Member

@samcoe samcoe left a comment

Thanks for the contribution! I added some tests and a little 💅

// Browser precedence
// 1. browser from config
// 2. BROWSER
func browserLauncher(f *cmdutil.Factory) string {

This comment has been minimized.

@samcoe

samcoe Aug 17, 2021
Member

I extracted this function from browser for testability purposes.

@mislav
mislav approved these changes Aug 17, 2021
Copy link
Member

@mislav mislav left a comment

This is all excellent. Thank you both!

pkg/cmd/factory/default_test.go Outdated Show resolved Hide resolved
pkg/cmd/factory/default.go Outdated Show resolved Hide resolved
@samcoe samcoe merged commit 8fb6bb6 into cli:trunk Aug 17, 2021
7 checks passed
7 checks passed
@github-actions
CodeQL-Build
Details
@github-actions
lint
Details
@github-actions
build (ubuntu-latest)
Details
@github-actions
build (windows-latest)
Details
@github-actions
build (macos-latest)
Details
@github-actions
build-minimum
Details
@github-code-scanning
CodeQL No new or fixed alerts
Details
The GitHub CLI automation moved this from Needs to be merged 🎉 to Pending Release 🥚 Aug 17, 2021
@despreston despreston deleted the despreston:858-config-browser branch Aug 17, 2021
@github-actions github-actions bot moved this from Pending Release 🥚 to Done 💤 in The GitHub CLI Aug 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
The GitHub CLI
  
Done 💤
Linked issues

Successfully merging this pull request may close these issues.

4 participants