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

using @nobles/hashes (with rebase) #174

Merged
merged 1 commit into from
Dec 9, 2023

Conversation

lenkan
Copy link
Collaborator

@lenkan lenkan commented Dec 8, 2023

I did the rebase and squash of #162 because we are keen to get rid of the wasm dependency of blake3.

@AlexAndrei98 you are still the author of the commit as you can see.

I will use my fork for now. It does not matter for me which PR is merged.

@lenkan lenkan changed the title using @nobles/hashes using @nobles/hashes (with rebase) Dec 8, 2023
Copy link

codecov bot commented Dec 8, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (cfb001b) 81.36% compared to head (febfcb0) 81.42%.

Files Patch % Lines
src/ready.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@               Coverage Diff               @@
##           development     #174      +/-   ##
===============================================
+ Coverage        81.36%   81.42%   +0.06%     
===============================================
  Files               46       46              
  Lines             4149     4141       -8     
  Branches          1030     1030              
===============================================
- Hits              3376     3372       -4     
+ Misses             741      737       -4     
  Partials            32       32              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@AlexAndrei98 AlexAndrei98 mentioned this pull request Dec 8, 2023
Copy link
Contributor

@AlexAndrei98 AlexAndrei98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lenkan Thank you for the rebase! I thought i had fixed mine. 🙏🏽

@lenkan
Copy link
Collaborator Author

lenkan commented Dec 8, 2023

@pfeairheller @rodolfomiranda Is this OK to merge? It works for our environment. Yesterday you mentioned that some people might have opinions on blake3 dependency, I don't recall the reasoning.

@lenkan lenkan merged commit 858dea2 into WebOfTrust:development Dec 9, 2023
5 checks passed
@lenkan lenkan deleted the noble-hashes-lenkan branch December 9, 2023 08:41
@@ -1,13 +1,5 @@
import _sodium from 'libsodium-wrappers-sumo';

export const ready: () => Promise<void> = async () => {
try {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome! Nice work

@kentbull
Copy link
Collaborator

kentbull commented Dec 9, 2023

I really hope this works. I'll be testing it out today. I assume it does.

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.

3 participants