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 a --no-confirmation option #158

Merged
merged 2 commits into from
Oct 23, 2023
Merged

Add a --no-confirmation option #158

merged 2 commits into from
Oct 23, 2023

Conversation

AltGr
Copy link
Contributor

@AltGr AltGr commented Oct 19, 2023

This might be required for some complex release pipelines where the package is already guaranteed to be well reviewed and checked before opam-publish is called.

I took care to add some safeguards to avoid copy-paste of ill-advised one-liners that would result in people submitting unreviewed releases and increasing the strain on the repository maintainers.

This might be required for some complex release pipelines where the package is
already guaranteed to be well reviewed and checked before `opam-publish` is
called.

I took care to add some safeguards to avoid copy-paste of ill-advised one-liners
that would result in people submitting unreviewed releases and increasing the
strain on the repository maintainers.
@AltGr
Copy link
Contributor Author

AltGr commented Oct 19, 2023

Closes #132

src/publishMain.ml Outdated Show resolved Hide resolved
@kit-ty-kate
Copy link
Contributor

Thanks

@kit-ty-kate kit-ty-kate merged commit d3f8a1f into master Oct 23, 2023
1 check passed
@kit-ty-kate kit-ty-kate deleted the no-confirmation branch October 23, 2023 18:27
@MisterDA
Copy link

MisterDA commented Oct 23, 2023

Could you explain why that different from OPAMYES=1 or a simple -y option? Other examples may include pacman's --noconfirm, apt-get's DEBIAN_FRONTEND=noninteractive, pip's --yes or --no-input.
EDIT: is it just because we're in a plugin, and that would collide with opam?

@kit-ty-kate
Copy link
Contributor

Could you explain why that different from OPAMYES=1 or a simple -y option?

OPAMYES is not supported by opam-publish, and in my opinion, does not provide an acceptable interface for such a dangerous option. It wouldn’t be a good option for currently existing docker images for example where OPAMYES is already set to true, so if someone were to run opam-publish in it, it would skip all the manual verification necessary.
Also, being an environment variable makes it hard to make sure accidental publishing doesn’t happen.

-y is too short and obscure as to its behaviour in my opinion for that particular case. It’s fine for lower stake actions but automated publishing is something else.

--no-confirmation is long enough that users should think twice about copy-pasting code that contains it and explicit enough for anyone reading it to know what it’s going to do.

@erikmd
Copy link

erikmd commented Oct 28, 2023

Dear all, thanks a lot for implementing this 👍👍

Just a small question to opam-publish maintainers: do you think this enhancement would deserve a release?

(otherwise, no worries: users could just as well do some opam git pin to benefit from it meanwhile)

Thanks!

@AltGr
Copy link
Contributor Author

AltGr commented Oct 30, 2023

@erikmd definitely, we'll do a release

@kit-ty-kate
Copy link
Contributor

@erikmd done: ocaml/opam-repository#24714

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.

Feature wish: Add a --yes flag to automatically accept Confirm questions
4 participants