Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Copy backoff module from Trillian #1424

Merged
merged 3 commits into from Jan 3, 2020
Merged

Copy backoff module from Trillian #1424

merged 3 commits into from Jan 3, 2020

Conversation

@gdbelvin
Copy link
Collaborator

gdbelvin commented Jan 3, 2020

A little copying is better than a little dependency

  • Go Proverb
@gdbelvin gdbelvin requested a review from google/keytransparency as a code owner Jan 3, 2020
@googlebot googlebot added the cla: yes label Jan 3, 2020
internal/backoff_test.go Outdated Show resolved Hide resolved
@codecov

This comment has been minimized.

Copy link

codecov bot commented Jan 3, 2020

Codecov Report

Merging #1424 into master will decrease coverage by 0.01%.
The diff coverage is 82.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1424      +/-   ##
==========================================
- Coverage   66.33%   66.31%   -0.02%     
==========================================
  Files          53       54       +1     
  Lines        3986     4026      +40     
==========================================
+ Hits         2644     2670      +26     
- Misses        952      962      +10     
- Partials      390      394       +4
Impacted Files Coverage Δ
internal/backoff/backoff.go 82.5% <82.5%> (ø)
core/sequencer/trillian_client.go 58.57% <0%> (-4.29%) ⬇️
impl/sql/directory/storage.go 67.66% <0%> (-1.51%) ⬇️
core/sequencer/server.go 72.96% <0%> (-0.66%) ⬇️

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 850d311...245f58a. Read the comment docs.

internal/backoff.go Outdated Show resolved Hide resolved
Copy link
Contributor

pavelkalinnikov left a comment

LGTM % file location and spelling warning.

@gdbelvin gdbelvin force-pushed the gdbelvin:backkoff branch from 4f80b5d to 245f58a Jan 3, 2020
@gdbelvin gdbelvin merged commit 2559372 into google:master Jan 3, 2020
5 checks passed
5 checks passed
GolangCI No issues found!
Details
Travis CI - Pull Request Build Passed
Details
cla/google All necessary CLAs are signed
codecov/patch 82.5% of diff hit (target 66.33%)
Details
codecov/project Absolute coverage decreased by -0.01% but relative coverage increased by +16.16% compared to 850d311
Details
@gdbelvin gdbelvin deleted the gdbelvin:backkoff branch Jan 3, 2020
gdbelvin added a commit to gdbelvin/keytransparency that referenced this pull request Jan 3, 2020
* master:
  Copy backoff module from Trillian (google#1424)
// See the License for the specific language governing permissions and
// limitations under the License.

// Package backoff allows retrying an operation with backoff.

This comment has been minimized.

Copy link
@NatalieDoduc

NatalieDoduc Jan 3, 2020

Is there any way of denoting that this is a copy of code that is shared in Trillian (and vice versa)? I notelic lid your copy vs dependency proverb, but the potential to divergence in both flavours is worrisome.

While here - was there not a public backoff or retry library that was suitable?

This comment has been minimized.

Copy link
@gdbelvin

gdbelvin Jan 3, 2020

Author Collaborator

Divergence is less of a risk here because we can fully test it in this project without worrying about breaking other projects. Perhaps I'm missing something though.

grpc-go has a backoff library but it is also inside an internal directory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.