-
Notifications
You must be signed in to change notification settings - Fork 10
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
Assorted chores #166
Assorted chores #166
Conversation
80a455a
to
ccdd5e6
Compare
ccdd5e6
to
52efe01
Compare
Codecov Report
@@ Coverage Diff @@
## main #166 +/- ##
==========================================
+ Coverage 77.95% 78.01% +0.05%
==========================================
Files 23 23
Lines 5053 5066 +13
==========================================
+ Hits 3939 3952 +13
Misses 1114 1114
|
f4905e5
to
58902b7
Compare
5c5f156
to
2afd0fa
Compare
@piotr-roslaniec what do you think of |
@cygnusv Makes sense, also |
@@ -1,5 +1,5 @@ | |||
[package] | |||
name = "group-threshold-cryptography-pre-release" | |||
name = "ferveo-tdec" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, so refreshing!
ferveo-tdec/README.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
README still refers to tpke
@@ -5,7 +5,7 @@ use ark_ec::pairing::Pairing; | |||
use criterion::{ | |||
black_box, criterion_group, criterion_main, BenchmarkId, Criterion, | |||
}; | |||
use group_threshold_cryptography_pre_release::{ | |||
use ferveo_tdec::{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This remind me of a situation a couple of months ago that was making me crazy when working with Ferveo until I realized (or actually, you pointed out). Even though the package and directory are called ferveo-tdec
, the crate is automatically renamed to ferveo_tdec
. Do you think it would be worth to rename everything to underscores?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using both -
and _
as a separator in crate names is fine. There is a debate on setting the naming convention.
I'm fine with renaming all crates, we have to republish them anyway. The directories should remain named according to the kebab-case
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've documented this as a todo here: #122
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The directories should remain named according to the kebab-case
Why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I can't cite any specific source on that, but I think it's a cargo
project convention. I think, very generally speaking, top-level modules (including *.rs
files) are to use kebab-case
and non-top-level files (never directories) are to use snake_case
.
2a75b9c
to
58002f5
Compare
24d100d
to
87c5f34
Compare
Type of PR:
Required reviews:
What does it do:
Issues fixed/closed:
Eq
forDkgPublicKey
#157.pyi
file linting #141