sir-lancebot icon indicating copy to clipboard operation
sir-lancebot copied to clipboard

Misleading docstring and variable names in `githubinfo.py`

Open Ibrahim2750mi opened this issue 2 years ago • 2 comments

Description

on_message function: https://github.com/python-discord/sir-lancebot/blob/9ecf423268bb3a89e881e4b28e5ee56fe9b33819/bot/exts/utilities/githubinfo.py#L175 https://github.com/python-discord/sir-lancebot/blob/9ecf423268bb3a89e881e4b28e5ee56fe9b33819/bot/exts/utilities/githubinfo.py#L183

Here the on_message function returns more than just issues, it also returns pull requests. So the docstring and the variable names should be changed accordingly.

Possible Solutions

  • For the docstring it could be changed to:
    • "Automatic issue and pull request linking."
  • For the variable name:
    • repo_sequential_numbers
    • issues_prs

Would you like to implement a fix?

  • [x] I'd like to implement the fix

Ibrahim2750mi avatar Feb 20 '23 16:02 Ibrahim2750mi

From GitHub's API's perspective, pull requests are a type of issue https://docs.github.com/en/rest/issues/issues?apiVersion=2022-11-28#list-issues-assigned-to-the-authenticated-user

Note: GitHub's REST API considers every pull request an issue, but not every issue is a pull request. ...

For user facing (e.g. help command text) I think it's worth listing both, but because this is internal I'm not sure it's worth changing, although maybe it would be more clear, I don't mind.

wookie184 avatar Feb 20 '23 22:02 wookie184

For user facing (e.g. help command text) I think it's worth listing both, but because this is internal I'm not sure it's worth changing, although maybe it would be more clear, I don't mind.

@wookie184 When I was trying to find the source of this command I overlooked this file because the docstring mentioned 'issues' in the on_messagefunction, I found the source of this through Chris when he redirected me. So it kinda causes confusion.

Ibrahim2750mi avatar Feb 21 '23 06:02 Ibrahim2750mi