- Notifications
You must be signed in to change notification settings - Fork30
[SHIPKIT-513] Automated commit message should have CI build ID#709
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
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!
| publicvoidapply(finalProjectproject) { | ||
| finalShipkitConfigurationconf =project.getPlugins().apply(ShipkitConfigurationPlugin.class).getConfiguration(); | ||
| StringcommitMessage =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
| publicvoidapply(finalProjectproject) { | ||
| finalShipkitConfigurationconf =project.getPlugins().apply(ShipkitConfigurationPlugin.class).getConfiguration(); | ||
| StringcommitMessage =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
| importorg.shipkit.gradle.configuration.ShipkitConfiguration | ||
| classCommitMessageUtilsTestextendsSpecification { |
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.