tumblr / colossus

I/O and Microservice library for Scala
Apache License 2.0
1.14k stars 96 forks source link

Fix publish script for snapshots #626

Closed aliyakamercan closed 6 years ago

aliyakamercan commented 6 years ago

@amotamed @jlbelmonte @benblack86 @DanSimon @dxuhuang

Checking -SNAPSHOT git tag.

I am not sure about this. Maybe we should release snapshots with each merge to develop (_-SNAPSHOT) ?

Any preferences?

codecov-io commented 6 years ago

Codecov Report

Merging #626 into develop will decrease coverage by 0.14%. The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #626      +/-   ##
===========================================
- Coverage    84.91%   84.76%   -0.15%     
===========================================
  Files           99       98       -1     
  Lines         4460     4432      -28     
  Branches       370      366       -4     
===========================================
- Hits          3787     3757      -30     
- Misses         673      675       +2
Impacted Files Coverage Δ
colossus/src/main/scala/colossus/core/Worker.scala 76.99% <0%> (-1.77%) :arrow_down:
...s/src/main/scala/colossus/core/server/Server.scala 94.36% <0%> (-0.08%) :arrow_down:
.../src/main/scala/colossus/core/ServerSettings.scala 100% <0%> (ø) :arrow_up:
...c/main/scala/colossus/core/ConnectionLimiter.scala

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 41c6d99...d7986e6. Read the comment docs.

benblack86 commented 6 years ago

What is TRAVIS_BRANCH?

aliyakamercan commented 6 years ago

It is the tag in this case. For PRs there are 2 builds; PR branch and the branch it is being merged to.

DanSimon commented 6 years ago

Personally I don't think think auto-publishing a snapshot with merges to develop is particularly useful, so I'm fine with dropping that. Someone asked for it a long time ago and I don't know if anyone has actually used it.

But that said, what's the reason for making this change? I don't think we should be publishing very many snapshots anyway. It's easy enough to build and publish Colossus locally if you want to test a change you just made. And we should be publishing milestone/candidate releases often enough for any pre-release testing.

aliyakamercan commented 6 years ago

@dan Yes, I guess we can release milestones and releases candidates of off develop. Regardless

($TRAVIS_PULL_REQUEST == "false" && $TRAVIS_BRANCH == "develop" && $(cat version.sbt) =~ "-SNAPSHOT")

this piece of code is wrong since branch cannot be develop on a non-pr build. I will remove that bit altogether, we will only be able to make milestone, rc and regular releases.

aliyakamercan commented 6 years ago

@benblack86 @DanSimon please take a look again.