Skip to content
This repository has been archived by the owner on Feb 11, 2022. It is now read-only.

Gradle: Adding support for snapshots #106

Merged
merged 12 commits into from May 28, 2018
Merged

Gradle: Adding support for snapshots #106

merged 12 commits into from May 28, 2018

Conversation

tomsammons
Copy link

This PR adds support for snapshots builds

These will be built off the development branch

We updated the gradle-build-properties-plugin to the latest version

Here is the link to the Jenkins job: https://ci.novoda.com/job/gradle-static-analysis-plugin-snapshot/

Paired with @tobiasheine

@@ -1,6 +1,6 @@
class GradlePlugins {
final bintrayRelease = 'com.novoda:bintray-release:0.4.0'
final buildProperties = 'com.novoda:gradle-build-properties-plugin:0.2'
final buildProperties = 'com.novoda:gradle-build-properties-plugin:0.4.1'
Copy link
Author

Choose a reason for hiding this comment

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

Our build properties was not working so we have updated to the latest version

Mecharyry
Mecharyry previously approved these changes May 25, 2018
Copy link
Contributor

@rock3r rock3r left a comment

Choose a reason for hiding this comment

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

Couple minor things but a-ok otherwise


apply plugin: 'com.gradle.plugin-publish'
apply plugin: 'com.novoda.bintray-release'
publish {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: empty line before publish

Copy link
Author

Choose a reason for hiding this comment

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

grgit.push {
tags = true
boolean isDryRun = buildProperties.cli['dryRun'].or(true).boolean
if (!isDryRun) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we invert? if (isDryRun) { ... } else { ... }

Copy link
Author

Choose a reason for hiding this comment

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

@tomsammons
Copy link
Author

I'm not so familiar with Jenkins setups so if somebody could take a look over this that would be cool

rock3r
rock3r previously approved these changes May 25, 2018
@rock3r
Copy link
Contributor

rock3r commented May 25, 2018

@tomsammons what about Jenkins? this PR doesn't seem to do anything Jenkins directly

@tomsammons
Copy link
Author

@rock3r Yep sorry, nothing about Jenkins

apply plugin: 'com.novoda.build-properties'
buildProperties {

secrets {
file(rootProject.file('secrets.properties'), '''
Copy link
Contributor

Choose a reason for hiding this comment

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

could you please change this too now that you're at it?

using rootProject.file('secrets.properties')
description = ''' ... '''

boolean isDryRun = cli['dryRun'].or(true).boolean
return isDryRun ?
['bintrayRepo': 'n/a', 'bintrayUser': 'n/a', 'bintrayKey': 'n/a'] :
new File("${System.getenv('BINTRAY_PROPERTIES')}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you defined the CI job to set this var up?

Copy link
Contributor

Choose a reason for hiding this comment

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

tags = true
}
}
boolean isDryRun = buildProperties.cli['dryRun'].or(true).boolean
Copy link
Contributor

Choose a reason for hiding this comment

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

this is used in 2 places already. could you extract as var in the outmost scope?

Copy link
Contributor

Choose a reason for hiding this comment

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

What exactly do you have in mind?
We consume this value the first time within the configuration of the buildProperties extension itself.
We could define it within the ext block with a default value and override it it within the buildPropeties.bintray configuration. But this way we would need to rely on the order when reading the value a second time.

As alternative we could extract a method isDryRun().

Copy link
Contributor

Choose a reason for hiding this comment

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

extracting a method in this gradle file sounds good too (no need to expose it outside using any ext)

} else {
dependsOn publishArtifact
boolean isSnapshot = buildProperties.cli['bintraySnapshot'].or(false)
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

Tom Sammons added 2 commits May 28, 2018 13:32
}

boolean isSnapshot() {
buildProperties.cli['bintraySnapshot'].or(false)
Copy link
Contributor

Choose a reason for hiding this comment

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

you're missing .boolean at the end of this like (see isDryRun())

@tomsammons tomsammons merged commit 24052a6 into develop May 28, 2018
@tomsammons tomsammons deleted the gradle/snapshots branch May 28, 2018 11:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants