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.

Sign up for GitHub

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

Jump to bottom

The library should follows the same idioms and conventions of the rest of our repos #29

Closed
ZER0 opened this issue Oct 23, 2021 · 0 comments
Closed

The library should follows the same idioms and conventions of the rest of our repos #29

ZER0 opened this issue Oct 23, 2021 · 0 comments
Assignees
@ZER0
Labels
team:WebTech End to End UX / UI and blockchain type:enhancement Issues concerning code or feature improvement (performance, refactoring, etc)

Comments

@ZER0
Copy link
Contributor

ZER0 commented Oct 23, 2021

Describe what you want implemented
The code base should be consistent with the rest of our repos.

Describe "Why" this is needed
The current code is not following the same idioms and conventions used by the rest of our code. This is misleading and makes harder to work across dusk's code, especially if it provides similar features. See for example https://github.com/dusk-network/dusk-pki

Describe alternatives you've considered
N/A

Additional context
N/A
@ZER0 ZER0 self-assigned this Oct 23, 2021
@ZER0 ZER0 added team:WebDev Web development comprising of UI and apps type:enhancement Issues concerning code or feature improvement (performance, refactoring, etc) team:WebTech End to End UX / UI and blockchain and removed team:WebDev Web development comprising of UI and apps labels Oct 23, 2021
ZER0 added a commit that referenced this issue Oct 25, 2021
@ZER0
Change the codebase to be more idiomatic and follows conventions
cb90859 
- Remove `Canonical` and `canon` flag

This library is not supposed to be compiled to WASM or be used by any
contracts.

- Implement `to_bytes` and `from_bytes` methods from `Serializable` trait
- Remove `serializable_size()` method in favor of `T::SIZE` from `Serializable` trait
- Add `dusk_bytes::Error` conversion to `Error`
- Use `core::fmt::Display` instead of `std::fmt::Display`
- Change `SecretKey::new` to `SecretKey::random` to be consistent
- Remove the patch version from `dusk_bytes` dependency in `Cargo.toml`
- Change integration tests to be outside `src` as they should be

Resolves: #29
@ZER0 ZER0 mentioned this issue Oct 25, 2021
Merged
ZER0 added a commit that referenced this issue Oct 25, 2021
@ZER0
Change the codebase to be more idiomatic and follows conventions
cf60b7b 
- Remove `Canonical` and `canon` flag

This library is not supposed to be compiled to WASM or be used by any
contracts.

- Implement `to_bytes` and `from_bytes` methods from `Serializable` trait
- Remove `serializable_size()` method in favor of `T::SIZE` from `Serializable` trait
- Add `dusk_bytes::Error` conversion to `Error`
- Use `core::fmt::Display` instead of `std::fmt::Display`
- Change `SecretKey::new` to `SecretKey::random` to be consistent
- Remove the patch version from `dusk_bytes` dependency in `Cargo.toml`
- Change integration tests to be outside `src` as they should be

Resolves: #29
ZER0 added a commit that referenced this issue Oct 25, 2021
@ZER0
Change the codebase to be more idiomatic and follows conventions
646ad38 
- Remove `Canonical` and `canon` flag

This library is not supposed to be compiled to WASM or be used by any
contracts.
To be fair we probably won't need to have this library as `no_std`
either (we also have only tests for `std`), but I kept as `no_std`
for future reference, since it helped to some some issue related to
compile a static library on `no_std` environment.

- Implement `to_bytes` and `from_bytes` methods from `Serializable` trait
- Remove `serializable_size()` method in favor of `T::SIZE` from `Serializable` trait
- Add `dusk_bytes::Error` conversion to `Error`
- Use `core::fmt::Display` instead of `std::fmt::Display`
- Change `SecretKey::new` to `SecretKey::random` to be consistent
- Remove the patch version from `dusk_bytes` dependency in `Cargo.toml`
- Change integration tests to be outside `src` as they should be

Resolves: #29
ZER0 added a commit that referenced this issue Oct 25, 2021
@ZER0
Change the codebase to be more idiomatic and follows conventions
cae17ab 
- Remove `Canonical` and `canon` flag

This library is not supposed to be compiled to WASM or be used by any
contracts.
To be fair we probably won't need to have this library as `no_std`
either (we also have only tests for `std`), but I kept as `no_std`
for future reference, since it helped to some some issue related to
compile a static library on `no_std` environment.

- Implement `to_bytes` and `from_bytes` methods from `Serializable` trait
- Remove `serializable_size()` method in favor of `T::SIZE` from `Serializable` trait
- Add `dusk_bytes::Error` conversion to `Error`
- Use `core::fmt::Display` instead of `std::fmt::Display`
- Change `SecretKey::new` to `SecretKey::random` to be consistent
- Remove the patch version from `dusk_bytes` dependency in `Cargo.toml`
- Change integration tests to be outside `src` as they should be
- Add `rust-toolchain` to ensure CI and local dev behave the same
- Remove overrides from the CI (it will use the `rust-toolchain`)
- Change `rustfmt.toml` to have the same settings of our repos

Resolves: #29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ZER0 ZER0

Labels
team:WebTech End to End UX / UI and blockchain type:enhancement Issues concerning code or feature improvement (performance, refactoring, etc)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@ZER0