-
Notifications
You must be signed in to change notification settings - Fork 21
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 --exclude to exclude the given packages from the publishing process #161
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would simplify the type for ~exclude
to be just string list
, but more importantly I think the code should check that the exclusions refer to packages found (that would mean, for example, that --exclude opam-admni
will fail instead of continuing to publish opam-admin
anyway.
done |
OpamConsole.error_and_exit `Not_found | ||
"Excluded package '%s' not found" | ||
(OpamPackage.Name.to_string name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why errorring instead of a warning and continue ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was a request from David (see above). I think it's fair to error on this since it avoid issues for users later down the line if they do not see the warning or if the opam-publish is used as part of an automated system
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm! Indeed, i looked at the code, didn't see David comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Fixes #144