Skip to content

feat!: Drop edx-name-affirmation as a dependency.#37385

Merged
feanil merged 3 commits intomasterfrom
feanil/drop_name_affirmation
Nov 17, 2025
Merged

feat!: Drop edx-name-affirmation as a dependency.#37385
feanil merged 3 commits intomasterfrom
feanil/drop_name_affirmation

Conversation

@feanil
Copy link
Copy Markdown
Contributor

@feanil feanil commented Sep 24, 2025

This dependency is specific to edx.org and should not be in the default
version of the edx-platform. There is related code in edx-platform but
circuit breakers should keep that from running for now until we can
clean it up.

Resolves #37598

BREAKING CHANGE: If you are relying on the edx-name-affirmation app
working, you should install it yourself before running the platform.

This dependency is specific to edx.org and should not be in the default
version of the edx-platform.  There is related code in edx-platform but
circuit breakers should keep that from running for now until we can
clean it up.

BREAKING CHANGE: If you are relying on the edx-name-affirmation app
working, you should install it yourself before running the platform.
…stalled

The service is specific to 2U and should not be installed by default.  When we
try to patch objects from that library, that can only be done if the
original object is importable.  So those tests don't make sense to have
in the base system which you should be able to run without the
edx-name-affirmation library.
@feanil feanil force-pushed the feanil/drop_name_affirmation branch from a09cd44 to 86032c1 Compare October 30, 2025 18:32
This should prevent it from accidentally getting added back to the
default build.
@feanil feanil marked this pull request as ready for review October 30, 2025 20:05
@feanil feanil requested a review from a team as a code owner October 30, 2025 20:05
@feanil feanil requested a review from robrap October 30, 2025 20:05
@robrap
Copy link
Copy Markdown
Contributor

robrap commented Oct 31, 2025

@feanil: This seems like a good place to use a fast-track DEPR. It would give a wider inform, and clarify that the community has no intention on providing a replacement for this (e.g. pluggable solution, etc.). Are you certain no one else would need to be informed?

In all cases, I appreciate you pinging us. I love that I don't have to block merges due to immediate concerns, but I still have some longer term concerns. 😄

For example, there is a DEPR WG question about what or where you plan to remove 2U-only functionality without someone providing an alternative. Note that Axim engineers may have created some edx.org functionality that is now 2U-only, and can anyone just decide to remove any of it whenever you want? When Is product review required to decide? Other possibilities? Thanks.

@robrap
Copy link
Copy Markdown
Contributor

robrap commented Oct 31, 2025

@feanil: Is this final clean-up of earlier work that just wasn't linked to?

@feanil
Copy link
Copy Markdown
Contributor Author

feanil commented Nov 3, 2025

Good call on the DEPR, I'll make that and add it here. There was no prior work as far as I know. I think at some point 2U had said that it should be possible to drop the dependency but no-one did any work about it. This is just me following up on this cleanup. There is a longer term cleanup of the actual import code and platform dependence that I'd love to talk with you about at the next DEPR meeting. I'd like to set a timeline for removing it from the platform but have whoever needs it(2U) do the work of extracting it from the platform to use events, signals, etc. That's a big ask so I want to make a reasonable timeline for it so that will be a separate DEPR.

@feanil
Copy link
Copy Markdown
Contributor Author

feanil commented Nov 6, 2025

FYI, the DEPR is here: #37598

I'll merge this on 2025/11/17

@feanil feanil merged commit 93bb80b into master Nov 17, 2025
53 checks passed
@feanil feanil deleted the feanil/drop_name_affirmation branch November 17, 2025 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[DEPR]: edx-name-affirmations as a default dependency

3 participants