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

[Docs] randomize_color now has its own docstring #3491

Merged
merged 5 commits into from Feb 22, 2020
Merged

[Docs] randomize_color now has its own docstring #3491

merged 5 commits into from Feb 22, 2020

Conversation

Dav-Git
Copy link
Contributor

@Dav-Git Dav-Git commented Feb 2, 2020

Type

  • Bugfix
  • Enhancement
  • New feature

Description of the changes

This closes #3468

@Dav-Git Dav-Git requested a review from Twentysix26 as a code owner Feb 2, 2020
@jack1142 jack1142 added Category: Docs Category: Utility Functions labels Feb 2, 2020
@Dav-Git
Copy link
Contributor Author

Dav-Git commented Feb 6, 2020

Well... some new checks showed up

@jack1142
Copy link
Member

jack1142 commented Feb 7, 2020

You need to update PR for them to trigger, something like merging/rebasing on top of V3/develop and then pushing would work.
Or you could make an empty commit...

git commit --allow-empty -m "Trigger CI"

A minor inconvenience for old PRs that gives us a lot better (faster) checks.

@jack1142
Copy link
Member

jack1142 commented Feb 21, 2020

I feel like it might be better to just remove randomize_color from docs and only leave randomize_colour (and consequently the docstring with alias shows only the correct function since the other won't be shown separately in docs).
To be clear, that's just my opinion.

@Dav-Git
Copy link
Contributor Author

Dav-Git commented Feb 21, 2020

Something I actually didn't think about which makes a lot of sense however. I agree. Will update the pr to this tomorrow

@jack1142 jack1142 added this to the 3.3.2 milestone Feb 21, 2020
@Dav-Git
Copy link
Contributor Author

Dav-Git commented Feb 22, 2020

Actually, how would I go about removing it from the docs? I've tried changing

.. automodule:: redbot.core.utils.embed
    :members:

to

.. automodule:: redbot.core.utils.embed.randomize_colour
    :members:

In framework_utils.rst

@jack1142
Copy link
Member

jack1142 commented Feb 22, 2020

@Dav-Git IMO, the best way to change would be changing from:

.. automodule:: redbot.core.utils.embed
    :members:

to

.. automodule:: redbot.core.utils.embed
    :members:
    :exclude-members: randomize_color

This way, if someone adds a new util to redbot.core.utils.embed, they won't have to modify the rst file to have it added to documentation.

@Dav-Git Dav-Git requested review from palmtree5 and tekulvw as code owners Feb 22, 2020
@Dav-Git Dav-Git requested a review from jack1142 Feb 22, 2020
@jack1142 jack1142 self-assigned this Feb 22, 2020
Copy link
Member

@jack1142 jack1142 left a comment

LGTM

@jack1142 jack1142 merged commit 0397401 into Cog-Creators:V3/develop Feb 22, 2020
5 checks passed
@jack1142 jack1142 changed the title [Docs] randomize_color now has it's own docstring [Docs] randomize_color now has its own docstring Feb 22, 2020
@Dav-Git Dav-Git deleted the patch-2 branch Feb 23, 2020
@jack1142 jack1142 added the Type: Bug label Jul 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Docs Category: Utility Functions Type: Bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants