-
-
Notifications
You must be signed in to change notification settings - Fork 422
Solves #345 : optional signing #346
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
Solves #345 : optional signing #346
Conversation
* Made gpgPassphrase optional for publishing * Added a flag to remove signing of published artifacts altogether
43015ac to
63e49f9
Compare
|
@rockjam want to review this? Since this is mostly your code |
|
Sure, will do |
|
|
||
| def publish(sonatypeCreds: String, | ||
| gpgPassphrase: String, | ||
| gpgPassphrase: String = "", |
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.
Is it possible to make this argument optional? Same applies to publishAll command
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 don't think it's possible to write a ScoptReader for "absence of values" tbh. However your suggestion to default to null is better than empty string. Addressed : ec362d8
| sonatypeSnapshotUri, | ||
| sonatypeCreds, | ||
| gpgPassphrase, | ||
| Some(gpgPassphrase).filter(_.nonEmpty), |
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.
If having gpgPassphrase parameter optional is not possible, I guess we could have default value as null and wrap it in Option(gpgPassphrase) instead of filtering out an empty string, what do you think?
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.
Addressed ec362d8
| case Some(passphrase) => | ||
| %("gpg", "--passphrase", passphrase, "--batch", "--yes", "-a", "-b", fileName) | ||
| case None => | ||
| %("gpg", "--batch", "--yes", "-a", "-b", fileName) |
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.
Is it useful to sign artifacts without gpgPassphrase ?
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 honestly don't know if it's useful, but I'm following Haoyi's 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.
Oh, I didn't notice that. All good then
|
@Baccata left some comments |
better than using empty string as default param.
|
@Baccata everything seems to be fine. Will merge once CI goes green |
#345