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 --no-browser flag #110

Merged
merged 3 commits into from
Sep 18, 2020
Merged

Add --no-browser flag #110

merged 3 commits into from
Sep 18, 2020

Conversation

phated
Copy link
Contributor

@phated phated commented Sep 1, 2020

Ref #109

This adds an option called --open-browser that a user can set to false to disable the open browser logic in a headless environment.

@phated
Copy link
Contributor Author

phated commented Sep 9, 2020

Anything I can do to help move this forward?

@lefessan
Copy link

My understanding is that this PR does not change the default behavior, which is to open a browser, but provides an option to disable it. Maybe the option should be more explicit, like --no-browser ?

@phated
Copy link
Contributor Author

phated commented Sep 10, 2020

Sure, I can make that change! I was thinking about making it --no-browser but thought it might be nice to have the option of changing the default in the future if automated publishes became the norm.

@phated
Copy link
Contributor Author

phated commented Sep 14, 2020

@lefessan Just made those changes. Is this what you were looking for?

@phated phated changed the title Add --open-browser option Add --no-browser flag Sep 14, 2020
Copy link
Member

@dra27 dra27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slight style-nit and harden the code not to leave a dangling block at the end of an if

src/publishSubmit.ml Outdated Show resolved Hide resolved
src/publishSubmit.ml Outdated Show resolved Hide resolved
@phated
Copy link
Contributor Author

phated commented Sep 17, 2020

Sorry for the delay here (I've been in interviews all week). I've updated the PR as per the review. Thanks!

@phated phated requested a review from dra27 September 17, 2020 18:13
Copy link
Member

@dra27 dra27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - @rjbou?

@rjbou
Copy link
Contributor

rjbou commented Sep 18, 2020

Good for me too! Thanks

@rjbou rjbou merged commit 407046a into ocaml-opam:master Sep 18, 2020
@phated phated deleted the open-browser-option branch December 21, 2020 06:41
@rjbou rjbou modified the milestone: 2.1.0 Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants