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

[PLATFORM-860]: Veil memory optimisations and refactoring #60

Conversation

WilliamVenner
Copy link
Contributor

https://prima-assicurazioni-spa.myjetbrains.com/youtrack/issue/PLATFORM-860

Veil has a large opportunity to optimise memory usage by writing directly to the std::fmt::Formatter, this should have been done from the start really.

I've also used this opportunity to refactor internal Veil code, as it was getting a bit messy and unclear.

@WilliamVenner WilliamVenner requested a review from a team as a code owner December 15, 2022 13:51
src/private.rs Outdated Show resolved Hide resolved
Co-authored-by: MaeIsBad <26093674+MaeIsBad@users.noreply.github.com>
veil-macros/src/lib.rs Show resolved Hide resolved
veil-macros/src/lib.rs Show resolved Hide resolved
veil-macros/src/flags.rs Outdated Show resolved Hide resolved
src/private.rs Show resolved Hide resolved
src/private.rs Outdated Show resolved Hide resolved
Copy link
Member

@MaeIsBad MaeIsBad left a comment

Choose a reason for hiding this comment

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

Seems moslty good for me

Partial,

/// Whether to redact with a fixed width, ignoring the length of the data.
Fixed(NonZeroU8),
Copy link
Member

Choose a reason for hiding this comment

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

any reason to keep this as just an u8 instead of u32 or usize?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think anyone is gonna need anything greater than 255 characters :V

@cpiemontese
Copy link
Contributor

Can this be merged @WilliamVenner @MaeIsBad?

@WilliamVenner
Copy link
Contributor Author

Yes I think it's ok :) just need approve

@cpiemontese cpiemontese self-requested a review December 16, 2022 14:18
@WilliamVenner WilliamVenner merged commit dd9d994 into master Dec 16, 2022
@WilliamVenner WilliamVenner deleted the PLATFORM-860/user-story/veil-memory-optimisations-and-refactoring branch December 16, 2022 14:18
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