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

Deprecate bad function plugin arg #9736

Merged

Conversation

Booplicate
Copy link
Member

@Booplicate Booplicate commented Nov 12, 2022

This is waiting for #9725, do not merge until then, it uses the new deprecation API.

Originally planned to remove this with r8, but I think it will be better if we give time and properly deprecate this parameter now.

Changes:

  • in the following funcs, changed the _args/args parameter from [] to None (which then being set to an empty tuple). Mutable def arguments can cause bugs
    • functionplugin
    • registerFunction
    • setArgs
  • setArgs is now deprecated
  • getArgs is now deprecated

Testing:

  • verify you get deprecation warning when using the parameter or the setArgs function

mutable args are bad
`functools.partial` exists and provides a more clear way to give both args and kwargs to callables
@Booplicate Booplicate added awaiting testing code needs to be tested DONT MERGE srs awaiting code review someone needs to check for syntax/logic/indentation errors high priority labels Nov 12, 2022
@Booplicate Booplicate added this to the 0.12.13 milestone Nov 12, 2022
@ThePotatoGuy
Copy link
Member

changed the _args parameter from [] to () (mutable def arguments can cause bugs)

for these, make it None in the params and do a None check to default - better than still leaving the door open for default arg weirdness.

Co-Authored-By: ThePotatoGuy <potato@desu.zone>
@ThePotatoGuy ThePotatoGuy modified the milestones: 0.12.13, ? 0.12.14 Dec 18, 2022
@ThePotatoGuy ThePotatoGuy modified the milestones: ? 0.12.14, ? 0.12.15 Feb 12, 2023
multimokia
multimokia previously approved these changes Jun 11, 2023
@multimokia multimokia dismissed their stale review June 14, 2023 03:29

The merge-base changed after approval.

@multimokia multimokia removed the DONT MERGE srs label Jun 14, 2023
multimokia
multimokia previously approved these changes Jun 15, 2023
@multimokia multimokia removed awaiting testing code needs to be tested awaiting code review someone needs to check for syntax/logic/indentation errors labels Jun 18, 2023
@multimokia multimokia merged commit bd44e0d into Monika-After-Story:content Jun 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants