Skip to content

Conversation

@daniellevass
Copy link
Contributor

@daniellevass daniellevass commented Jul 27, 2020

Description of the pull request

When we added the TweetNaclFast dependency, we added a lot of things we're not using (we're just decrypting, we're not encrypting any messages) - I've removed all the things I don't think we're using.
...

Why is the change necessary?

It reduces the amount of code other people need to install from us - it also improves our code coverage metric.
...


@codecov-commenter
Copy link

codecov-commenter commented Jul 27, 2020

Codecov Report

Merging #275 into master will increase coverage by 30.82%.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff              @@
##             master     #275       +/-   ##
=============================================
+ Coverage     45.43%   76.25%   +30.82%     
  Complexity      284      284               
=============================================
  Files            31       31               
  Lines          3053     1798     -1255     
  Branches        254      137      -117     
=============================================
- Hits           1387     1371       -16     
+ Misses         1610      371     -1239     
  Partials         56       56               
Impacted Files Coverage Δ Complexity Δ
...a/com/pusher/client/crypto/nacl/TweetNaclFast.java 93.61% <ø> (+60.10%) 26.00 <0.00> (ø)

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 4626265...01c9f8c. Read the comment docs.

@daniellevass daniellevass self-assigned this Jul 27, 2020
@daniellevass daniellevass marked this pull request as ready for review July 27, 2020 15:18
@daniellevass daniellevass requested a review from elverkilde July 27, 2020 15:19
@daniellevass daniellevass merged commit fc50dbe into master Jul 31, 2020
@daniellevass daniellevass deleted the dev-crypto-coverage branch July 31, 2020 14:29
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.

4 participants