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

[BD-21] Deprecate waffle namespaces #80

Merged
merged 1 commit into from
Jan 8, 2021

Conversation

regisb
Copy link
Contributor

@regisb regisb commented Nov 3, 2020

Description: Deprecate WaffleFlagNamespace and WaffleSwitchNamespace. All related features and methods are displaced to the WaffleFlag/WaffleSwitch classes.

JIRA: https://openedx.atlassian.net/wiki/spaces/COMM/pages/1596358943/BD-21+Toggles+Settings+Documentation

Reviewers:

Merge checklist:

  • All reviewers approved
  • CI build is green
  • Version bumped
  • Changelog record added
  • Documentation updated (not only docstrings)
  • Commits are squashed

Post merge:

  • Create a tag
  • Check new version is pushed to PyPi after tag-triggered build is
    finished.
  • Delete working branch (if not needed anymore)

This PR depends on #98

@openedx-webhooks openedx-webhooks added blended PR is managed through 2U's blended developmnt program waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Nov 3, 2020
@openedx-webhooks
Copy link

openedx-webhooks commented Nov 3, 2020

Thanks for the pull request, @regisb! I've created BLENDED-650 to keep track of it in Jira. More details are on the BD-21 project page.

When this pull request is ready, tag your edX technical lead.

@regisb regisb force-pushed the regisb/deprecate-namespaces branch from 2ee6951 to 6a2154c Compare November 3, 2020 11:54
@regisb regisb changed the title [BD-21] Deprecate waffle namespaces (WIP) [BD-21] Deprecate waffle namespaces Nov 5, 2020
@openedx-webhooks openedx-webhooks added needs triage and removed waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Nov 5, 2020
@regisb
Copy link
Contributor Author

regisb commented Nov 5, 2020

Following our conversation @robrap this is ready for review.

@regisb regisb force-pushed the regisb/deprecate-namespaces branch from 6a2154c to 7b8acdd Compare November 5, 2020 13:39
@robrap
Copy link
Contributor

robrap commented Nov 5, 2020

@regisb: FYI: I am going to let the dust settle on the earlier PR first. Please update relevant comments (Changelog, PR Description, etc.) that details the backward incompatible change on the interfaces of WaffleFlag and WaffleSwitch. For example, dropping the namespace from the constructor, and possibly mentioning the properties/methods that are going away, etc.

@regisb regisb force-pushed the regisb/deprecate-namespaces branch 2 times, most recently from 31d537a to c6cdfdd Compare November 11, 2020 17:31
@regisb regisb force-pushed the regisb/deprecate-namespaces branch 3 times, most recently from 719d638 to f825560 Compare November 19, 2020 10:19
@regisb regisb force-pushed the regisb/deprecate-namespaces branch 2 times, most recently from f853271 to fd675fa Compare December 14, 2020 14:57
Copy link
Contributor

@robrap robrap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @regisb. I must just be super picky. :)

CHANGELOG.rst Outdated Show resolved Hide resolved
CHANGELOG.rst Outdated Show resolved Hide resolved
CHANGELOG.rst Outdated Show resolved Hide resolved
CHANGELOG.rst Outdated Show resolved Hide resolved
CHANGELOG.rst Outdated Show resolved Hide resolved
edx_toggles/toggles/__future__.py Show resolved Hide resolved
edx_toggles/toggles/__future__.py Show resolved Hide resolved
edx_toggles/toggles/__init__.py Outdated Show resolved Hide resolved
edx_toggles/toggles/internal/waffle/legacy.py Outdated Show resolved Hide resolved
edx_toggles/toggles/internal/waffle/legacy.py Outdated Show resolved Hide resolved
Copy link
Contributor

@robrap robrap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very close. Thanks. Reminder that this is blocked adjusting the custom attributes and ensuring monitoring is happy before merging/releasing.

CHANGELOG.rst Outdated Show resolved Hide resolved
CHANGELOG.rst Outdated Show resolved Hide resolved
edx_toggles/toggles/__future__.py Show resolved Hide resolved
edx_toggles/toggles/__init__.py Outdated Show resolved Hide resolved
Copy link
Contributor

@robrap robrap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nits if you want to update, but this is ready to go. Monitoring shows we should be ready for 2.0.0.

Unfortunately, deprecated_incompatible_legacy_waffle_class is too new, so we don't have any old data to prove that it ever worked. It also doesn't have tests. However, if we assume it works as planned, then we are all set. I am getting data for deprecated_compatible_legacy_waffle_class, so hopefully that is a good enough indicator.

CHANGELOG.rst Outdated Show resolved Hide resolved
CHANGELOG.rst Outdated Show resolved Hide resolved
CHANGELOG.rst Outdated Show resolved Hide resolved
@regisb regisb force-pushed the regisb/deprecate-namespaces branch from 156e68c to 5efe207 Compare January 7, 2021 10:19
We now expose the old namespace classes as LegacyWaffle*. This causes a
breaking change of import path.
@regisb regisb force-pushed the regisb/deprecate-namespaces branch from 5efe207 to d028d0b Compare January 7, 2021 10:19
@robrap
Copy link
Contributor

robrap commented Jan 7, 2021

@regisb: Any reason you didn't merge, or just because you needed to wait for green build?

@regisb
Copy link
Contributor Author

regisb commented Jan 8, 2021

Sorry, I thought you would merge it! I'll make the release now.

@regisb regisb merged commit 03bb0ba into openedx:master Jan 8, 2021
@regisb regisb deleted the regisb/deprecate-namespaces branch January 8, 2021 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blended PR is managed through 2U's blended developmnt program merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants