X Tutup
Skip to content

Get rid of os.environment access in default arguments#48

Merged
seratch merged 1 commit intoslackapi:mainfrom
seratch:remove-env-access-in-default-values
Aug 27, 2020
Merged

Get rid of os.environment access in default arguments#48
seratch merged 1 commit intoslackapi:mainfrom
seratch:remove-env-access-in-default-values

Conversation

@seratch
Copy link
Contributor

@seratch seratch commented Aug 27, 2020

This pull request improves the internal code by removing os.environment access in default arguments. Although I removed most of such code in early development, it seems I failed to completely remove them at that time.

Default parameter values are evaluated from left to right when the function definition is executed. This means that the expression is evaluated once, when the function is defined, and that the same “pre-computed” value is used for each call.

https://docs.python.org/3.8/reference/compound_stmts.html#function-definitions

Category (place an x in each of the [ ])

  • slack_bolt.App and/or its core components
  • slack_bolt.async_app.AsyncApp and/or its core components
  • Adapters in slack_bolt.adapter

Requirements (place an x in each [ ])

Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.

  • I've read and understood the Contributing Guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've run ./scripts/run_tests.sh after making the changes.

@seratch seratch added the bug Something isn't working label Aug 27, 2020
@seratch seratch added this to the 0.4.0a0 milestone Aug 27, 2020
@seratch seratch self-assigned this Aug 27, 2020
@seratch seratch merged commit ca2e25f into slackapi:main Aug 27, 2020
@seratch seratch deleted the remove-env-access-in-default-values branch August 27, 2020 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

X Tutup