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

Update finagle and other twitter deps to 2019 compatible versions. #361

Closed
wants to merge 10 commits into from

Conversation

cacts
Copy link

@cacts cacts commented Dec 12, 2019

The (still unreleased) develop version of storehaus is not binary compatible with the current versions of Finagle. This PR updates them to recent versions by using the twitter Bufs provided in the new finagle-x clients, and removes ChannelBuffers which were obsoleted with the release of netty 4.

@CLAassistant
Copy link

CLAassistant commented Mar 24, 2020

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ cactauz
❌ Don Smith III


Don Smith III seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@johnynek
Copy link
Collaborator

thanks for the PR, if we can get this green, I'll merge it.

@codecov-commenter
Copy link

codecov-commenter commented Jul 17, 2020

Codecov Report

Merging #361 into develop will increase coverage by 0.28%.
The diff coverage is 62.06%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #361      +/-   ##
===========================================
+ Coverage    61.21%   61.49%   +0.28%     
===========================================
  Files          119      119              
  Lines         1815     1792      -23     
  Branches       116      116              
===========================================
- Hits          1111     1102       -9     
+ Misses         704      690      -14     
Impacted Files Coverage Δ
...n/scala/com/twitter/storehaus/http/HttpStore.scala 79.16% <ø> (+8.79%) ⬆️
...twitter/storehaus/memcache/MemcacheLongStore.scala 0.00% <ø> (ø)
...scala/com/twitter/storehaus/redis/RedisStore.scala 38.88% <ø> (+5.55%) ⬆️
...com/twitter/storehaus/memcache/MemcacheStore.scala 34.61% <25.00%> (-9.39%) ⬇️
...cala/com/twitter/storehaus/mysql/ValueMapper.scala 62.50% <50.00%> (+7.73%) ⬆️
...er/storehaus/memcache/MergeableMemcacheStore.scala 81.81% <77.77%> (ø)
...cala/com/twitter/storehaus/WriteThroughStore.scala 80.00% <100.00%> (ø)
.../twitter/storehaus/mysql/MergeableMySqlStore.scala 86.95% <0.00%> (+4.34%) ⬆️

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 cb4c347...a8461bd. Read the comment docs.

@cacts
Copy link
Author

cacts commented Jul 17, 2020

thanks @johnynek! looks like i've finally got it fixed up.

edit: the mima previous versions will have to be uncommented after an artifact is published.

@jho
Copy link

jho commented Jul 31, 2020

Any chance we can get this merged?

@cacts
Copy link
Author

cacts commented Sep 9, 2020

@johnynek mind having a look at this?

@cacts cacts closed this Dec 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants