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

Enhance BaseToggle with additional enabled/disabled toggle methods #290

Open
3 tasks
robrap opened this issue Jul 3, 2023 · 11 comments
Open
3 tasks

Enhance BaseToggle with additional enabled/disabled toggle methods #290

robrap opened this issue Jul 3, 2023 · 11 comments
Labels
good first issue A good task for a newcomer to start with help wanted Ready to be picked up by anyone in the community

Comments

@robrap
Copy link
Contributor

robrap commented Jul 3, 2023

Enhance BaseToggle with additional methods.

Add the following:

  • is_disabled: Returns not is_enabled.
  • is_toggled_on: Returns is_enabled.
  • is_toggled_off: Returns not is_enabled.

This should make a variety of conditions more readable when ENABLE_ or DISABLE_ are in the toggle name.

Example 1: is_disabled

# BEFORE: slightly less readable
not SOME_FEATURE_TOGGLE.is_enabled()

# AFTER: more clear
SOME_FEATURE_TOGGLE.is_disabled()  

Example 2: is_toggled_off

# BEFORE: brain twister
not DISABLE_SOME_FEATURE.is_enabled()

# AFTER: more clear
DISABLE_SOME_FEATURE.is_toggled_off()  

Notes:

Questions:

  • Is this in fact simpler to understand? Are there better options?
    • We've decided to move forward

Work required:

@robrap robrap added help wanted Ready to be picked up by anyone in the community good first issue A good task for a newcomer to start with labels Jul 3, 2023
@Theo-flux
Copy link

Hello @robrap can i work on this ?

@robrap
Copy link
Contributor Author

robrap commented Jul 6, 2023

That would be great @Theo-flux. Let me know if you have any questions.

@robrap
Copy link
Contributor Author

robrap commented Jul 6, 2023

@Theo-flux: As part of this work, it might be good to share the proposal and get feedback, in case anyone has any better ideas on naming. Would you want to start a discussion in discuss.openedx.org about this proposal?

@Theo-flux
Copy link

That would be great @Theo-flux. Let me know if you have any questions
Let me go through the codebase. I'll get back to you.

@Theo-flux
Copy link

@Theo-flux: As part of this work, it might be good to share the proposal and get feedback, in case anyone has any better ideas on naming. Would you want to start a discussion in discuss.openedx.org about this proposal?

Just opened a discussion on openedx now https://discuss.openedx.org/t/ideas-for-naming-methods-in-a-class/10630/1

@robrap
Copy link
Contributor Author

robrap commented Jul 7, 2023

Thank you! Also, it will be pretty quick to search/replace your code if someone suggest a good change in the next day or two, so feel free to start the PR in parallel.

@Theo-flux
Copy link

OK @robrap sounds great! Let me set things up from end. I'll let you know when I have question(s) to ask. Thanks

@Theo-flux
Copy link

@robrap kindly check my PR. Not all checks passed. I already filled the contribution form. I am yet to get the document so I can sign on it. Thanks!

@Theo-flux
Copy link

Theo-flux commented Jul 10, 2023

@robrap I am still yet to receive the CLA document. I hope this is not an error

@robrap
Copy link
Contributor Author

robrap commented Jul 10, 2023

@Theo-flux: Once the CLA gets worked out (which it will), I can do the review. Thank you!

@robrap
Copy link
Contributor Author

robrap commented Dec 5, 2023

[inform] This ticket is available for work. @Theo-flux was almost able to complete this in this PR: #293. It just needs some minor work to get it over the line. Thanks again @Theo-flux for your contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue A good task for a newcomer to start with help wanted Ready to be picked up by anyone in the community
Projects
None yet
Development

No branches or pull requests

2 participants