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

Misleading docstring and variable names in githubinfo.py #1213

Open
1 task done
Ibrahim2750mi opened this issue Feb 20, 2023 · 2 comments
Open
1 task done

Misleading docstring and variable names in githubinfo.py #1213

Ibrahim2750mi opened this issue Feb 20, 2023 · 2 comments
Labels
type: bug Something isn't working

Comments

@Ibrahim2750mi
Copy link
Contributor

Ibrahim2750mi commented Feb 20, 2023

Description

on_message function:

Automatic issue linking.

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?

  • I'd like to implement the fix
@Ibrahim2750mi Ibrahim2750mi added the type: bug Something isn't working label Feb 20, 2023
@wookie184
Copy link
Contributor

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.

@Ibrahim2750mi
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants