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

DOCS: Improve docstring for "frappe.client.get_value" #29040

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

rahulporuri
Copy link

Explain the details for making this change. What existing problem does the pull request solve?

This commit improves the existing docstring for the frappe.client.get_value API. Specifically, it adds information about all of the available arguments and provides additional information about the return values and potential exceptions. Examples are also provided in the docstring now.

Screenshots/GIFs

The rendered docstring looks as follows

image

This commit improves the existing docstring for the frappe.client.get_value
API. Specifically, it adds information about all of the available arguments
and provides additional information about the return values and potential
exceptions. Examples are also provided in the docstring now
@github-actions github-actions bot added the add-test-cases Add test case to validate fix or enhancement label Jan 3, 2025
@rahulporuri
Copy link
Author

@ankush could you please help me here? i'm not sure if you're one of the code owners here but I'm not sure who else to ping (pinging you because you're the author of the most recent merged PR)

A little bit of context - I intend to slowly improve docstrings across the entire framework. I plan to open one PR a day/few PRs a week because I'd prefer keeping the PRs small to make reviews quick and easy. Unless the maintainers don't prefer this approach. If necessary, i'd be happy to do one PR per week, updating docstrings in a few functions at a time, but I'm not comfortable with that approach.

@akhilnarang
Copy link
Member

@rahulporuri the other approach of a larger PR covering more functions would be preferable.


This does look good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
add-test-cases Add test case to validate fix or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants