Skip to content

Improved comments for clarity and details :) #4565

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

Merged
merged 4 commits into from Nov 20, 2024
Merged

Improved comments for clarity and details :) #4565

merged 4 commits into from Nov 20, 2024

Conversation

ghost
Copy link

@ghost ghost commented Nov 15, 2024

Improved comments for clarity and details in payment bot example:

  • Added specific explanations for optional parameters in invoice functions
  • Clarified purpose of ShippingQuery, PreCheckoutQuery, and payment callbacks
  • Enhanced readability of logging and bot setup sections

@Poolitzer Poolitzer changed the base branch from master to doc-fixes November 15, 2024 17:46
@Poolitzer Poolitzer added the ⚙️ examples affected functionality: examples label Nov 15, 2024
Copy link
Member

@Bibo-Joshi Bibo-Joshi left a comment

Choose a reason for hiding this comment

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

Hi. Thanks very much for your PR! AFAIK you've already discussed the changes with Poolitzer. I've added a few additional comments below. None of them are critical.
Please also have a look at the failing pre-commit checks. They are due to a few lines exceeding the line length limit of 99 chars.

@@ -21,41 +21,44 @@
logging.basicConfig(
format="%(asctime)s - %(name)s - %(levelname)s - %(message)s", level=logging.INFO
)
# set higher logging level for httpx to avoid all GET and POST requests being logged
# Set a higher logging level for httpx to avoid logging every GET and POST request
Copy link
Member

Choose a reason for hiding this comment

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

this comment is used in all examples. If it's changed, I would like to have it adapted in the other examples as well

Copy link
Author

Choose a reason for hiding this comment

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

I'm unsure whether I should revert it or apply the same change to all the other examples.

Copy link
Member

Choose a reason for hiding this comment

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

Your call :) I'm fine with either solution. Thanks for the other updates!

Copy link
Author

Choose a reason for hiding this comment

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

Well, I've changed it back to the original because I believe both versions explain the purpose well.

Copy link
Member

@Bibo-Joshi Bibo-Joshi left a comment

Choose a reason for hiding this comment

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

Thanks for the update! Now only the pre-commit failure is remaining:)

@ghost
Copy link
Author

ghost commented Nov 20, 2024

Passed ;)

@Bibo-Joshi Bibo-Joshi merged commit d66fabc into python-telegram-bot:doc-fixes Nov 20, 2024
1 of 2 checks passed
@Bibo-Joshi
Copy link
Member

Thank you very much for the contribution!

@Bibo-Joshi Bibo-Joshi mentioned this pull request Nov 24, 2024
1 task
@github-actions github-actions bot locked and limited conversation to collaborators Nov 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
⚙️ examples affected functionality: examples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants