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.

Sign up for GitHub

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

Jump to bottom

rename set_scope and redefine_scope #23

Closed
1 of 2 tasks
da-h opened this issue Jul 14, 2020 · 5 comments · May be fixed by #57
Closed
1 of 2 tasks

rename set_scope and redefine_scope #23

da-h opened this issue Jul 14, 2020 · 5 comments · May be fixed by #57
Assignees
@da-h
Milestone
v2.0

Comments

@da-h
Copy link
Owner

da-h commented Jul 14, 2020 •
edited
Loading

Both commands sound very similar, however their used cases are pretty different.
What this actually does should be more visible in the method name.

Also, add unit-tests for both functions:

  • set_scope
  • register_as
@da-h
Copy link
Owner Author

da-h commented Nov 11, 2020

Ideas for replacement names for set_scope:

  • use_state(module_id)
  • share_state(module_id)
  • use_state_from(module_id)

Ideas for replacement names for redefine_scope:

  • rename_module(module_id)
  • rename(module_id)
  • register_module_as(module_id)

Any thoughts @sbrodehl ?

@da-h da-h self-assigned this Nov 11, 2020
@da-h da-h added this to the v2.0 milestone Nov 11, 2020
@sbrodehl
Copy link
Collaborator

I like use_state(module_id) for set_scope but with the addition, that a error/warning is thrown, when module_id does not exist.
As a replacement for redefine_scope I prefer register_as(module_id) (or redefine_as).

Anyway, their use cases should be added in the docs.

@da-h da-h mentioned this issue Dec 7, 2020
Merged
4 tasks
@da-h da-h linked a pull request Dec 7, 2020 that will close this issue
Rework of module inheritance #48
Merged
4 tasks
@da-h
Copy link
Owner Author

da-h commented Dec 7, 2020

If we rename set_scope we should also rename the positional argument scope=... inside register_defaults.

Any ideas for a consistent naming scheme?

Scope is maybe misleading? Maybe we should rename it to namespace?

@sbrodehl
Copy link
Collaborator

sbrodehl commented Dec 7, 2020

namespace sounds about right.
Maybe rename set_scope to use_namespace as well?

@da-h da-h removed a link to a pull request Dec 9, 2020
Rework of module inheritance #48
Merged
4 tasks
@da-h da-h linked a pull request Dec 30, 2020 that will close this issue
Draft: Register as / Replacing Modules from other Repositories #57
Draft
@da-h
Copy link
Owner Author

da-h commented Jan 4, 2021 •
edited
Loading

After looking more into this, I tend to maintain set_scope function unchanged.
redefine_scope, however has been changed in #57.

@da-h da-h closed this as completed Jan 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@da-h da-h

Labels
None yet
Projects
None yet
Milestone
v2.0
Development

Successfully merging a pull request may close this issue.

Draft: Register as / Replacing Modules from other Repositories da-h/miniflask
2 participants
@sbrodehl @da-h