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

[DEPR]: Extraneous XBlock Mixins #714

Closed
kdmccormick opened this issue Jan 31, 2024 · 3 comments
Closed

[DEPR]: Extraneous XBlock Mixins #714

kdmccormick opened this issue Jan 31, 2024 · 3 comments
Assignees
Labels
depr Proposal for deprecation & removal per OEP-21

Comments

@kdmccormick
Copy link
Member

kdmccormick commented Jan 31, 2024

Proposal Date

2024-01-31

Target Ticket Acceptance Date

2024-02-09

Earliest Open edX Named Release Without This Functionality

Redwood - 2024-04

Rationale

The implementations of the XBlock and XBlockAside classes are chopped into about a dozen tiny mixins. This was done many years ago, probably for some mix of reasons including code organization and theoretical reusability. Now, in 2024 in the openedx GitHub org, there are uses of the XBlock, XBlockAside, and XBlockMixin classes, but we do not see uses of any of the other classes or mixins (other than one extraneous reference to HandlersMixin in edx-platform, which can be removed).

Unfortunately, these mixins make it harder/impossible to write robust type constraints for XBlock and XBlockAside. They also make it harder to find code in the XBlock package. See discussion.

Removal

In an upcoming XBlock 2.0.0 release...

  • Removed: class xblock.core.SharedBlockBase.
  • Removed: module xblock.mixins, including...
    • these classes, collapsed into xblock.core.Blocklike:
      • xblock.mixins.RuntimeServicesMixin
      • xblock.mixins.ScopedStorageMixin
      • xblock.mixins.IndexInfoMixin
      • xblock.mixins.XmlSerializationMixin
      • xblock.mixins.HandlersMixin
    • these classes, collapsed into xblock.core.XBlock:
      • xblock.mixins.ChildrenModelMetaclass
      • xblock.mixins.HierarchyMixin
      • xblock.mixins.ViewsMixin
  • New: class xblock.core.Blocklike, serving as a shared base for xblock.core.XBlockMixin, xblock.core.XBlock, and xblock.core.XBlockAsidse.
  • No functional changes to xblock.plugin.Plugin, xblock.core.XBlock and xblock.core.XBlockAside.
  • Backwards-compatible enhancements to xblock.core.XBlockMixin include the functionality of these former classes:
    • xblock.mixins.IndexInfoMixin
    • xblock.mixins.XmlSerializationMixin
    • xblock.mixins.HandlersMixin

The resulting class hierarchy will look like this:

# xblock/plugin.py:
class Plugin:
   ... # No changes.

# xblock/core.py:
class Blocklike:
   ... # Collapse in: RuntimeServicesMixin, ScopedStorageMixin, XmlSerializationMixin, HandlersMixin, IndexInfoMixin
class XBlockMixin(Blocklike):
   ... # No changes (but XmlSerializationMixin, HandlersMixin, and IndexInfoMixin have been added via Blocklike).
class XBlockAside(Blocklike, Plugin):
   ... # No changes.
class XBlock(Blocklike, Plugin):
   ... # Collapse in: HierarchyMixin, ViewMixin

Replacement

See "Removal"

Deprecation

No plan.

Migration

See "Removal"

Additional Info

No response

Task List

No response

@github-actions github-actions bot added the depr Proposal for deprecation & removal per OEP-21 label Jan 31, 2024
@kdmccormick kdmccormick self-assigned this Jan 31, 2024
@kdmccormick
Copy link
Member Author

kdmccormick commented Jan 31, 2024

kdmccormick added a commit to kdmccormick/xblock that referenced this issue Feb 12, 2024
kdmccormick added a commit to kdmccormick/xblock that referenced this issue Feb 12, 2024
@kdmccormick
Copy link
Member Author

This has been accepted. Removal is in progress:

kdmccormick added a commit to kdmccormick/xblock that referenced this issue Feb 12, 2024
kdmccormick added a commit to kdmccormick/xblock that referenced this issue Feb 12, 2024
kdmccormick added a commit to kdmccormick/edx-platform that referenced this issue Feb 13, 2024
This has been a no-op for a long time anyway, since HierarchyMixin
has been explicitly mixed into XBlock for as long as I remember.

Ref: openedx/XBlock#714
kdmccormick added a commit to kdmccormick/edx-platform that referenced this issue Feb 13, 2024
This has been a no-op for a long time anyway, since HierarchyMixin
has been explicitly mixed into XBlock for as long as I remember.

Ref: openedx/XBlock#714
kdmccormick added a commit to kdmccormick/xblock that referenced this issue Feb 22, 2024
kdmccormick added a commit to kdmccormick/edx-platform that referenced this issue Feb 22, 2024
This has been a no-op for a long time anyway, since HierarchyMixin
has been explicitly mixed into XBlock for as long as I remember.

Ref: openedx/XBlock#714
kdmccormick added a commit to kdmccormick/xblock that referenced this issue Mar 18, 2024
Various extraneous classes have been removed from the XBlock API, simplifying its implementation
and making debugging of XBlock instances easier. We believe that most, if not all, XBlock API users
will be unaffected by this change. Some improvements have also been made to the reference documentation.

See CHANGELOG.rst for details.

Closes: openedx#714
kdmccormick added a commit to kdmccormick/xblock that referenced this issue Mar 18, 2024
Various extraneous classes have been removed from the XBlock API, simplifying its implementation
and making debugging of XBlock instances easier. We believe that most, if not all, XBlock API users
will be unaffected by this change. Some improvements have also been made to the reference documentation.

See CHANGELOG.rst for details.

Closes: openedx#714
kdmccormick added a commit to kdmccormick/xblock that referenced this issue Mar 18, 2024
Various extraneous classes have been removed from the XBlock API, simplifying its implementation
and making debugging of XBlock instances easier. We believe that most, if not all, XBlock API users
will be unaffected by this change. Some improvements have also been made to the reference documentation.

See CHANGELOG.rst for details.

Closes: openedx#714
kdmccormick added a commit to kdmccormick/edx-platform that referenced this issue Mar 18, 2024
This has been a no-op for a long time anyway, since HierarchyMixin
has been explicitly mixed into XBlock for as long as I remember.

Ref: openedx/XBlock#714
kdmccormick added a commit to kdmccormick/edx-platform that referenced this issue Mar 19, 2024
This has been a no-op for a long time anyway, since HierarchyMixin
has been explicitly mixed into XBlock for as long as I remember.

Ref: openedx/XBlock#714
kdmccormick added a commit to kdmccormick/edx-platform that referenced this issue Mar 19, 2024
This has been a no-op for a long time anyway, since HierarchyMixin
has been explicitly mixed into XBlock for as long as I remember.

Ref: openedx/XBlock#714
kdmccormick added a commit to kdmccormick/edx-platform that referenced this issue Mar 19, 2024
This has been a no-op for a long time anyway, since HierarchyMixin
has been explicitly mixed into XBlock for as long as I remember.

Ref: openedx/XBlock#714
@kdmccormick kdmccormick reopened this Mar 19, 2024
@kdmccormick
Copy link
Member Author

The XBlock PR has landed, but we are still working to apply the new XBlock version to edx-platform:

kdmccormick added a commit to kdmccormick/edx-platform that referenced this issue Apr 10, 2024
This has been a no-op for a long time anyway, since HierarchyMixin
has been explicitly mixed into XBlock for as long as I remember.

Ref: openedx/XBlock#714
kdmccormick added a commit to kdmccormick/edx-platform that referenced this issue Apr 16, 2024
This has been a no-op for a long time anyway, since HierarchyMixin
has been explicitly mixed into XBlock for as long as I remember.

Ref: openedx/XBlock#714
kdmccormick added a commit to openedx/edx-platform that referenced this issue Apr 17, 2024
This has been a no-op for a long time anyway, since HierarchyMixin
has been explicitly mixed into XBlock for as long as I remember.

Ref: openedx/XBlock#714
KyryloKireiev pushed a commit to raccoongang/edx-platform that referenced this issue Apr 24, 2024
This has been a no-op for a long time anyway, since HierarchyMixin
has been explicitly mixed into XBlock for as long as I remember.

Ref: openedx/XBlock#714
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
depr Proposal for deprecation & removal per OEP-21
Projects
Status: Removed
Development

No branches or pull requests

1 participant