-
Notifications
You must be signed in to change notification settings - Fork 35
[SHIPKIT-513] Automated commit message should have CI build ID #709
base: master
Are you sure you want to change the base?
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 suggested improvements. Thanks! Looking forward to merging!
@@ -55,13 +57,15 @@ | |||
|
|||
public void apply(final Project project) { | |||
final ShipkitConfiguration conf = project.getPlugins().apply(ShipkitConfigurationPlugin.class).getConfiguration(); | |||
String commitMessage = generateCommitMessagePostfix(conf, getProperty("TRAVIS_BUILD_NUMBER")); |
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.
Travis exports env variables rather than system properties.
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.
Good catch! Thanks
@@ -55,13 +57,15 @@ | |||
|
|||
public void apply(final Project project) { | |||
final ShipkitConfiguration conf = project.getPlugins().apply(ShipkitConfigurationPlugin.class).getConfiguration(); | |||
String commitMessage = generateCommitMessagePostfix(conf, getProperty("TRAVIS_BUILD_NUMBER")); |
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.
GitPlugin should be decoupled from Travis, so that we can use it for Circle CI, etc. Take a look at #514 - we're adding new code to TravisPlugin.
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.
That's why I exposed it to CommitMessageUtil but using Travis Plugin for that seems to be really better way
|
||
import org.shipkit.gradle.configuration.ShipkitConfiguration | ||
|
||
class CommitMessageUtilsTest extends Specification { |
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.
Nice test coverage! Thanks!
@mockito/shipkit-developers Thanks for the feedback :) I'll try to implement suggestion quickly |
@mockitoguy I did a few changes (it took some time but that was really busy week...), I still have a question what do you @mockito/shipkit-developers suggest is the best way of testing changes like that. I see there are not many automatic tests for interoperability of plugins and I'm looking for a better way than doing that manually - what itself is hard when it's touching Travis builds. Do you have any good idea? |
No description provided.