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

Add references to wiki #3306

Merged
merged 21 commits into from Oct 31, 2022
Merged

Add references to wiki #3306

merged 21 commits into from Oct 31, 2022

Conversation

lemontree210
Copy link
Member

@lemontree210 lemontree210 commented Oct 23, 2022

addresses #3110.

@lemontree210
Copy link
Member Author

If I add links to examples or Wiki not to the entire class, but to one of the args, do I add same links to both Args: and Attributes:?

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 for the PR! You found quite some places to put references, nice :) I left some comments where I thought that the added value of an additional reference is not so big (11 comments may sound like a lot, but compared to the number of your additions, that's only a few :D ).

If I add links to examples or Wiki not to the entire class, but to one of the args, do I add same links to both Args: and Attributes:?

I guess I'd make it dependent one which one is used more frequently by the user. Stupid example: a user usually has no reason to build an instance of Job manually, so in case you wanted to add something to Job.callback, I'd add it to the attribute, not the argument.

telegram/_bot.py Outdated Show resolved Hide resolved
telegram/_bot.py Outdated Show resolved Hide resolved
telegram/_message.py Outdated Show resolved Hide resolved
telegram/ext/_application.py Outdated Show resolved Hide resolved
telegram/ext/_application.py Outdated Show resolved Hide resolved
telegram/ext/_callbackqueryhandler.py Outdated Show resolved Hide resolved
telegram/ext/_contexttypes.py Outdated Show resolved Hide resolved
telegram/ext/_handler.py Show resolved Hide resolved
telegram/ext/_updater.py Outdated Show resolved Hide resolved
telegram/ext/filters.py Outdated Show resolved Hide resolved
@lemontree210
Copy link
Member Author

Thank you for your feedback!

I guess I'd make it dependent one which one is used more frequently by the user.

I was thinking the same way, but decided to ask with issue #3109 in mind (potential unification of args and attrs in docstrings). So OK, I'll proceed with this understanding.

@lemontree210
Copy link
Member Author

I finished adding stuff I had found :)

telegram/_chat.py Outdated Show resolved Hide resolved
telegram/ext/_callbackqueryhandler.py Outdated Show resolved Hide resolved
@lemontree210
Copy link
Member Author

Thank you for your comments! That should be it

Copy link
Member

@harshil21 harshil21 left a comment

Choose a reason for hiding this comment

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

good and thorough work! Found some things which can be further refined-

telegram/_bot.py Show resolved Hide resolved
telegram/ext/_application.py Outdated Show resolved Hide resolved
telegram/ext/_inlinequeryhandler.py Outdated Show resolved Hide resolved
telegram/ext/_jobqueue.py Show resolved Hide resolved
telegram/ext/_messagehandler.py Outdated Show resolved Hide resolved
telegram/ext/filters.py Outdated Show resolved Hide resolved
Comment on lines +1534 to +1535
.. seealso:: `Types of Handlers <https://github.com/\
python-telegram-bot/python-telegram-bot/wiki/Types-of-Handlers>`_
Copy link
Member

Choose a reason for hiding this comment

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

not needed imo

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess I'll have to wait for comment from @Bibo-Joshi here because in this comment there was a suggestion that I should remove links to 2 examples (which I did) but not to "Types of Handlers" wiki (so I left it there)

Copy link
Member

Choose a reason for hiding this comment

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

I'm fine with either way. You were very thorough on adding referencings, so we're bound to have a few references that may not be necessary (depending on who's judging :D), but a few more is surely better than less :)

Copy link
Member

Choose a reason for hiding this comment

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

ok eh let's keep it

@harshil21 harshil21 added this to the v20.0b1 milestone Oct 26, 2022
@harshil21 harshil21 added the hacktoberfest-accepted Label PRs with this so that Hacktoberfest counts them as valid label Oct 26, 2022
* Bot: move link to Your First Bot wiki
to class docstring

* Application: move link to Your First Bot
and Architecture wiki pages to class docstring

* InlineQueryHandler: remove link to Types of Handlers

* JobQueue: remove duplicate link to Timerbot example

* filters.py: remove link to Your First Bot

* MessageHandler: move link to Advanced Filters
from class docstring to `filters` arg
Copy link
Member

@harshil21 harshil21 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@Poolitzer Poolitzer left a comment

Choose a reason for hiding this comment

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

LGTM

@Poolitzer Poolitzer linked an issue Oct 31, 2022 that may be closed by this pull request
@harshil21 harshil21 merged commit 985203d into python-telegram-bot:doc-fixes Oct 31, 2022
@lemontree210 lemontree210 deleted the docs-references-to-examples-and-wiki-3110 branch October 31, 2022 09:14
@Poolitzer Poolitzer mentioned this pull request Oct 31, 2022
11 tasks
@github-actions github-actions bot locked and limited conversation to collaborators Nov 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation hacktoberfest-accepted Label PRs with this so that Hacktoberfest counts them as valid
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants