-
-
Notifications
You must be signed in to change notification settings - Fork 360
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
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 |
@@ -73,32 +73,35 @@ trait PublishModule extends JavaModule { outer => | |||
} | |||
|
|||
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
release: Boolean): define.Command[Unit] = T.command { | ||
val PublishModule.PublishData(artifactInfo, artifacts) = publishArtifacts() | ||
new SonatypePublisher( | ||
sonatypeUri, | ||
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