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

Large code/documentation cleanup #1476

Merged
merged 24 commits into from
Jul 24, 2022

Conversation

baronkobama
Copy link
Contributor

@baronkobama baronkobama commented Jul 8, 2022

Summary

This PR does a ton of things, but I'll attempt to summarize here:

  • Fixes a large amount of spelling and grammatical errors in code (not entirely necessary, but it removes linting typo/proofreading errors).
  • Fixes broken links on documentation (not all fixed, see below for remaining).
  • Polishes redundant code -- a few examples:
    • Remove _Undefined in favor of MISSING in abc.py.
    • Remove unused imports in several files.
    • Edit tuple returns to return val, val, ... instead of (val, val, ...).
  • Shortens lines that extend past L120.
  • Fixes some small bugs.
  • More small things I am probably glossing over.

Regarding broken links, there are still a few that remain and I have yet to get around to fixing yet. Namely:

  • Auto mod objects (i.e. AutoModRule, AutoModActionExecutionEvent, and more) that are referenced in the documentation (here and here from what I've found) are broken links as the objects are not currently listed anywhere in the documentation. (will be handled in a separate PR)
  • Any references to ForumChannel in the documentation are broken as it is not currently listed anywhere. (found here, here, here, here, and here so far.)
  • Not all members of TextChannel are being displayed on the docs, resulting in missing attributes on the object itself and a broken reference. (broken reference found here, haven't found any others so far.)

Also, lastly, this PR will probably remain in draft until the broken links listed above are fixed (any help is much appreciated, I can't do much more documentation work for my own sanity) and the changes made here are ensured to not cause any issues (once again, any testing done is much appreciated).

Information

  • This PR fixes an issue.
  • This PR adds something new (e.g. new method or parameters).
  • This PR is a breaking change (e.g. methods or parameters removed/renamed).
  • This PR is not a code change (e.g. documentation, README, typehinting, examples, ...).

Checklist

  • I have searched the open pull requests for duplicates.
  • If code changes were made then they have been tested.
    • I have updated the documentation to reflect the changes.
  • If type: ignore comments were used, a comment is also left explaining why.

@Lulalaby
Copy link
Member

Lulalaby commented Jul 8, 2022

Please don't say this should go into release too 😅

Lulalaby
Lulalaby previously approved these changes Jul 8, 2022
Copy link
Member

@Lulalaby Lulalaby left a comment

Choose a reason for hiding this comment

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

tbh LGTM so far, @Dorukyum what you think

@Lulalaby Lulalaby added this to the v2.1 milestone Jul 8, 2022
discord/bot.py Show resolved Hide resolved
discord/channel.py Outdated Show resolved Hide resolved
discord/channel.py Outdated Show resolved Hide resolved
discord/ext/commands/core.py Outdated Show resolved Hide resolved
discord/ext/commands/view.py Outdated Show resolved Hide resolved
discord/http.py Outdated Show resolved Hide resolved
@Lulalaby Lulalaby self-requested a review July 8, 2022 20:58
@EmmmaTech
Copy link
Contributor

Shortens lines that extend past L120.

Personally I think instead that another Black linting PR (that re-adds the CI with some differences) would be better for this instead of doing it in this PR.

discord/activity.py Outdated Show resolved Hide resolved
discord/cog.py Show resolved Hide resolved
discord/ext/pages/pagination.py Outdated Show resolved Hide resolved
discord/guild.py Outdated Show resolved Hide resolved
discord/guild.py Outdated Show resolved Hide resolved
discord/mentions.py Outdated Show resolved Hide resolved
discord/mentions.py Outdated Show resolved Hide resolved
discord/stage_instance.py Show resolved Hide resolved
discord/state.py Outdated Show resolved Hide resolved
discord/utils.py Outdated Show resolved Hide resolved
@baronkobama
Copy link
Contributor Author

Personally I think instead that another Black linting PR (that re-adds the CI with some differences) would be better for this instead of doing it in this PR.

Should this be done? I don't see a reason to undo those changes just so it can be done again, but if others think so then I can revert them.

@baronkobama baronkobama marked this pull request as ready for review July 10, 2022 19:33
discord/guild.py Outdated Show resolved Hide resolved
discord/guild.py Outdated Show resolved Hide resolved
@Lulalaby
Copy link
Member

I meant only featureable ^^
GitHub mobile sucks

I read your whole pr in bed because I couldn't sleep XD

@Lulalaby Lulalaby enabled auto-merge (squash) July 11, 2022 00:19
@Lulalaby Lulalaby added Merge with squash documentation Improvements or additions to documentation bug Something isn't working priority: high High Priority status: awaiting review Awaiting review from a maintainer labels Jul 11, 2022
@Lulalaby Lulalaby requested a review from Dorukyum July 17, 2022 19:39
@Lulalaby Lulalaby disabled auto-merge July 17, 2022 21:31
@Lulalaby Lulalaby enabled auto-merge (squash) July 17, 2022 21:31
@Lulalaby Lulalaby requested a review from Middledot July 17, 2022 21:32
Lulalaby
Lulalaby previously approved these changes Jul 17, 2022
Copy link
Member

@Lulalaby Lulalaby left a comment

Choose a reason for hiding this comment

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

Again I'm sleepy and once more I approve this pr as LGTM.
And because I only fixed a merge conflict YEEEEEEEEET

Copy link
Member

@Middledot Middledot left a comment

Choose a reason for hiding this comment

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

Otherwise lgtm

discord/commands/permissions.py Outdated Show resolved Hide resolved
discord/http.py Outdated Show resolved Hide resolved
discord/scheduled_events.py Outdated Show resolved Hide resolved
discord/stage_instance.py Outdated Show resolved Hide resolved
docs/api.rst Show resolved Hide resolved
auto-merge was automatically disabled July 22, 2022 20:17

Head branch was pushed to by a user without write access

@Middledot Middledot requested a review from Lulalaby July 24, 2022 21:01
@Lulalaby Lulalaby enabled auto-merge (squash) July 24, 2022 21:34
@Lulalaby Lulalaby merged commit 3fe9d18 into Pycord-Development:master Jul 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation priority: high High Priority status: awaiting review Awaiting review from a maintainer
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants