scrooge: Fixing Travis CI builds #196
Conversation
|
Ready @mosesn the build is now fixed at .travis.yml for non master branches by publishing local SNAPSHOT versions accordingly. |
| local name=$1 | ||
| local repo=$2 | ||
| local branch=$3 | ||
| git clone $repo || { |
There was a problem hiding this comment.
we can simplify this by using git clone "$repo" --branch "$branch"
There was a problem hiding this comment.
That's right ... I will go for it and change it at the script.
|
This is very cool, but I think if we use set -e we can avoid a lot of the manual error checking, since the tools themselves can explain what went wrong. We could potentially add -x too. What do you think? |
|
Thank you for your review !! 👍 😄 ... I'll reply on every comment of yours |
|
Yes, using set -e and even with -x will be a lot better, I will do that change. |
csaltos
changed the title
|
Ready !! 👍 😄 ... I've added the changes you proposed and the script is now cleaner and to the point. |
|
LGTM! Can you add a commit in the format described here: https://github.com/twitter/finagle/blob/master/CONTRIBUTING.md#style so it's easy to reason about when we merge it in? We should really have a CONTRIBUTING.md for scrooge too, but looks like we forgot to include one. @nshkrob @travisbrown could you guys sign off on this too? |
Problem Travis CI builds are broken because they are looking for SNAPSHOT versions of their dependencies and they are not found. Solution Deploy the SNAPSHOT versions locally cloning the remote repos and then using sbt publishLocal Result Now Travis CI builds are working using the locally published SNAPSHOT versions of their dependencies
|
Ready Moses, now I added a commit in the format you refer me. If there is something else to complete this PR, please let me know. |
| @@ -14,6 +14,10 @@ object Scrooge extends Build { | |||
| val suffix = if (branch == "master") "" else "-SNAPSHOT" | |||
|
|
|||
| val libVersion = "3.18.1" + suffix | |||
|
|
|||
| // For develop users: if you are using new features in util and finagle dependencies | |||
There was a problem hiding this comment.
Can we say "For users of the develop branch"? Seems a little clearer.
There was a problem hiding this comment.
Also, it might be a good idea to recommend publishing Util and Finagle locally for anyone working on the develop branch—users may not have any idea whether Scrooge is using features that have been introduced since the last release, and if they guess wrong they may end up with weird errors (possibly at runtime).
There was a problem hiding this comment.
Of course. Here it goes !!
There was a problem hiding this comment.
Ostrich is also a required local dependency when working with the develop branch.
There was a problem hiding this comment.
Anyway, I think that explanation is already included at https://github.com/twitter/scrooge/blob/master/README.md
csaltos
changed the title
|
LGTM, thanks for the patch! |
|
This looks great! Thanks much, @csaltos! |
|
You are very welcome Moses, Nik and Travis !! 👍 😄 |
Problem Travis CI builds are broken because they are looking for SNAPSHOT versions of their dependencies and they are not found. Solution Deploy the SNAPSHOT versions locally cloning the remote repos and then using sbt publishLocal Result Now Travis CI builds are working using the locally published SNAPSHOT versions of their dependencies
|
The previous build failed and it's because sometimes the builds got network errors while downloading external artifacts. If it's OK with you, I will try to activate cache for this external artifacts to keep the builds better and also to make them faster. It's something similar to the cache configuration at Finatra here: https://github.com/twitter/finatra/blob/master/.travis.yml |
Problem Some Travis CI build failed because they got network errors while trying to download external artifacts. Solution Cache the external artifacts at the Travis CI configuration Result With the externals artifacts cached the Travis CI builds will be faster and the network errors while downloading external artifacts will be diminished
|
Ready, the cache for external artifacts is now active, so now LGTM2 !! 😉 😄 |
|
I've pulled this in, should show up on develop in a few days. Thanks for the patch! |
|
Great, thank you Nik !! |
Problem
Travis CI builds are broken because they are looking for SNAPSHOT versions of their dependencies and they are not found.
Solution
Deploy the SNAPSHOT versions locally cloning the remote repos and then using sbt publishLocal
Result
Now Travis CI builds are working using the locally published SNAPSHOT versions of their dependencies