Skip to content

Conversation

chttrjeankr
Copy link
Contributor

a modified shift cipher algorithm with the explanation in the docstrings

Copy link
Member

@cclauss cclauss left a comment

Choose a reason for hiding this comment

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

We need at least one function or method to have doctests. See CONTRIBUTING.md.

Also, what about some type hints (also see CONTRIBUTING.md).

removed __make_one_digit()
@chttrjeankr chttrjeankr requested a review from cclauss October 23, 2019 16:06
@chttrjeankr
Copy link
Contributor Author

Thanks for helping out with this commit. 👍
I was a bit confused about how to frame the doctests. 😕

Any other changes needed for this PR or is everything set now?

Copy link
Member

@cclauss cclauss left a comment

Choose a reason for hiding this comment

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

Awesome work!! Congratulations. I made one minor mod on line 171 to test the return value.

Thanks much!

@cclauss cclauss merged commit d477a4d into TheAlgorithms:master Oct 23, 2019
@cclauss
Copy link
Member

cclauss commented Oct 23, 2019

This was our 1,000th PR closed.

@chttrjeankr
Copy link
Contributor Author

Loved the prompt response and reviews I got for my PRs for the last two days. 👍 This was one of my first useful contributions in the open-source community and the experience has left me wanting to contribute more and more. 😄

The 1,000th PR will be a milestone to remember for me. 🎉 🎉

@TheAlgorithms TheAlgorithms deleted a comment from tootsieTian Oct 23, 2019
@chttrjeankr chttrjeankr deleted the shuffled_shift_cipher branch October 23, 2019 18:36
stokhos pushed a commit to stokhos/Python that referenced this pull request Jan 3, 2021
* introduced shuffled_shift_cipher.py in /ciphers

* made requested changes

* introduced doctests, type hints
removed __make_one_digit()

* test_end_to_end() inserted

* Make test_end_to_end() a test ;-)
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.

2 participants