tiny functions added/implemented ( #139 )

[lvntky]

Hi, I added 2 tiny functions due the issue #139 . I did not implement the generate_symmetric_key function because I'm not sure that I'm on the right track, and I don't want to affect the project badly. And sorry if I do something silly :) this is my first ever PR. Anyway, if the implementation goes right I can start the implementing generate_symmetric_key function similar to this function.

[chester-leung]

Hi @lvntky this looks great! There's actually no need to implement a generate_symmetric_key() function in C++. The C++ should not be user facing - the user should only be calling Python functions.

We can continue to use this function, but replace the num_bytes variable with the result of calling _LIB.cipher_key_size() in Python (this calls the C++ function you wrote from Python). This is the only change necessary to fix this issue.

We should then remove the parameter from the generate_symmetric_key() function since we no longer need it to be passed in. Let me know if this makes sense or if you have any other questions!

[lvntky]

hi @chester-leung thank you for this great and informative explanation. So as I understand I make only changes on generate_symmetric_key() on the core.py file right? If it is right I will start working on it ASAP. Also thank you very much for your friendly and helpful response friend 🙏

[chester-leung]

@lvntky yes that's correct. You should only have to make a change in the generate_symmetric_key() function in core.py to call the C++ cipher_key_size() function you added in C++. Thank you!

[lvntky]

@chester-leung thank you very much! I'm on to it then

[lvntky]

hi @chester-leung I made a new contribution and small changes. I deleted the extern "C" int generate_symmetric_key(cipher_key_size()) {} in the c_api.cpp file since we don't need, and I change num_bytes variable with _LIB.cipher_key_size() in the core.py file. Sorry if I made something silly, as I said this is my first ever PR, and I really want to contribute to this project. And also thank you for your patience, informative and friendly instructions. If I made something wrong let me know and change it. Best wishes 🙏

[chester-leung]

@lvntky this looks great! Left a comment.

Please also fix the build (I think you're missing a semicolon here) and also run our pre-commit hooks discussed in step 4 here to format your code.

[chester-leung]

Can you please create a variable num_bytes and set it to _LIB.cipher_key_size() for clarity? This will add one more line but should make things easier to read.

[lvntky]

of course I'm doing it right now

[lvntky]

hi @chester-leung I add num_byte variable end assign it to _LIB.cipher_key_size() for better readability, as you said so.

[chester-leung]

Thanks! Please update us once you've fixed the build and reformatted.

[lvntky]

hi @chester-leung thank you for your patience and friendly instructions. i added a semicolon to cpp function and I run the pre-commit. Also, I added some comments to functions, I can delete it if you want. But I'm a little confused about the build, should I build with docker?

[chester-leung]

Please remove these comments

[chester-leung]

please remove this comment

[chester-leung]

Pre-commit essentially installs some git "hooks" that will reformat your code before you commit. I'm not sure if the code has been formatted correctly - in particular, there likely shouldn't be a space between = and num_bytes here

Here are some instructions

# Install pre-commit
pip3 install pre-commit

# in the root mc2 directory, install the pre-commit hooks
pre-commit install

Then, after you make a change to the code and git add/commit, you should see the following:

By build, I meant these checks

It looks like you're passing them now, great! Eventually we'll want to also add a linter check to enforce formatting (and to ensure that all contributors have run the pre-commit formatting hooks.

[lvntky]

hi @chester-leung i deleted the comments and run pre-commit. here is the screenshot:

[chester-leung]

This looks good! Just FYI, when you run pre-commit and it modifies some files, you'll have to readd the modified files to the git staging area. Formatting looks good for now though!

Once the checks pass, we can merge this in. Thanks for your contribution @lvntky !

[lvntky]

thank you very much for your friendly and informative information @chester-leung i am really glad to my first contribution is for mc2. I will hunting the issues again 😄

[chester-leung]

Sounds great! @lvntky if you're interested, #147 and #148 would be good to do next. Thank you again!

[lvntky]

yup I am totally interested friend, i have small work into job after that i dive to them 😄