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

Refactor function encode_attributes #776

Open
Patrik-Stas opened this issue Mar 15, 2023 · 1 comment
Open

Refactor function encode_attributes #776

Patrik-Stas opened this issue Mar 15, 2023 · 1 comment
Labels
good first issue Good for newcomers

Comments

@Patrik-Stas
Copy link
Contributor

Patrik-Stas commented Mar 15, 2023

In file encoding.rs function pub fn encode_attributes(attributes: &str) -> VcxResult<String> takes &str as argument - it then tries to parse it to Hashmap<String, serde_json::Value>.

Task:

  • change the function input and output types to be less generic than just &str and String - for example, it could look like
pub fn encode_attributes(attributes: CredentialAttributes) -> VcxResult<CredentialAttributesEncoded>

and hence making the function declaration communicate more about the input it requires and output it's producing.

Obivously, this will have impact on all of the code calling the function, so you will have to update that appropriately. The main place where encode_attributes is being called from. You could transitively continue applying this typing improvements on calling functions.

Additionally, you can try to refactor the function implementation itself as it's quite long.

@Patrik-Stas Patrik-Stas added the good first issue Good for newcomers label Mar 15, 2023
@Patrik-Stas Patrik-Stas changed the title Refactor function encode_attributes Refactor function signature encode_attributes Mar 15, 2023
@Patrik-Stas Patrik-Stas changed the title Refactor function signature encode_attributes Refactor function encode_attributes Mar 15, 2023
@Patrik-Stas
Copy link
Contributor Author

Patrik-Stas commented Apr 17, 2023

This issue has been picked up by Styvane in #778

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant