-
Notifications
You must be signed in to change notification settings - Fork 188
slice function's documentation made user friendly #456
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, looks good. I'd remove L207 (Deduction process:) and L212 (Extraction process:), I think they're redundant because the paragraphs that follow each already say what process is happening.
I added them so that a person can categorise a line under either of the 2 different processes. Explicitly grouping them makes it easier to get a bird view of the overall function and memorise what function does, otherwise there will be too many lines for a user under a single large umbrella to draw a map (in brain) of what function |
Sounds good, I don't think it's too important to remove them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thanks.
Ok. 3 approvals, I will merge it. Thank you @aman-godara for this PR. |
Previously, the documentation was bit lengthy and complex to understand especially for a person learning a programming language for the first time.
Although conceptually documentation is still same but it is simpler and shorter now.
Please let me know which one did you find easier to understand.