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

Fixed crash originated in bank.set_balance #2997

Merged
merged 11 commits into from Sep 22, 2019
Merged

Fixed crash originated in bank.set_balance #2997

merged 11 commits into from Sep 22, 2019

Conversation

Drapersniper
Copy link
Contributor

@Drapersniper Drapersniper commented Sep 12, 2019

Type

  • Bugfix
  • Enhancement
  • New feature

Description of the changes

Fixed crash seen in DM when calling bank.set_balance which was introduced in #2926

Drapersniper and others added 6 commits Aug 8, 2019
`[p]bankset maxbal` can be used to set the maximum bank balance

Signed-off-by: Guy <guyreis96@gmail.com>
Signed-off-by: guyre <27962761+drapersniper@users.noreply.github.com>
Signed-off-by: guyre <27962761+drapersniper@users.noreply.github.com>
Signed-off-by: guyre <27962761+drapersniper@users.noreply.github.com>
Signed-off-by: guyre <27962761+drapersniper@users.noreply.github.com>
Signed-off-by: guyre <27962761+drapersniper@users.noreply.github.com>
@Flame442
Copy link
Member

@Flame442 Flame442 commented Sep 13, 2019

If this is fixing a bug introduced in dev, the changelog should probably be misc.

Signed-off-by: guyre <27962761+drapersniper@users.noreply.github.com>
@Flame442 Flame442 self-assigned this Sep 18, 2019
Copy link
Member

@Flame442 Flame442 left a comment

The two functions affected by this change need to have their docstrings and typehints updated accordingly. Both now accept Union[discord.Member, discord.User] and both now raise RuntimeError (from the get_s they use) when a user is passed and the bank is server based.

Copy link
Collaborator

@Cog-CreatorsBot Cog-CreatorsBot left a comment

Changes requested by Flame#2941

Signed-off-by: guyre <27962761+drapersniper@users.noreply.github.com>
Signed-off-by: guyre <27962761+drapersniper@users.noreply.github.com>
@Drapersniper
Copy link
Contributor Author

@Drapersniper Drapersniper commented Sep 20, 2019

All sorted, thanks for the review Flame!

@Flame442
Copy link
Member

@Flame442 Flame442 commented Sep 20, 2019

The wording of the RuntimeError's description "guild was not provided" makes it seems like there should be a way to pass a guild to the functions. This should be changed to something that references how a user was used and a member is required.

Signed-off-by: guyre <27962761+drapersniper@users.noreply.github.com>
Copy link
Collaborator

@Cog-CreatorsBot Cog-CreatorsBot left a comment

Approved by Flame#2941

@Flame442 Flame442 added the QA: Passed label Sep 20, 2019
@mikeshardmind mikeshardmind added this to the 3.2.0 milestone Sep 22, 2019
@mikeshardmind mikeshardmind merged commit 575e55c into Cog-Creators:V3/develop Sep 22, 2019
1 check passed
@Drapersniper Drapersniper deleted the bank-bugfix branch Oct 11, 2019
@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
QA: Passed Type: Bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants