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

chore: remove unused backends DS-960 #208

Merged
merged 3 commits into from
Jun 5, 2024
Merged

Conversation

BryanttV
Copy link
Contributor

@BryanttV BryanttV commented May 31, 2024

Description

This PR removes unused backends.

The following settings were removed from common.py:

settings.GET_CERTIFICATES_MODULE = 'eox_tenant.edxapp_wrapper.backends.certificates_module_i_v1'
settings.UTILS_MODULE_BACKEND = 'eox_tenant.edxapp_wrapper.backends.util_h_v1'

Additionally, the following modules were removed from edxapp_wrapper/backends:

  • edx_auth_i_v1.py

All other backends are currently being used in common.py file.

Testing instructions

  1. Install the plugin with these changes in a Quince environment.
  2. Test the functionality of the plugin. Everything should work correctly,

Checklist for Merge

  • Tested in a remote environment
  • Updated documentation
  • Rebased master/main
  • Squashed commits

@Alec4r Alec4r requested review from Alec4r and Asespinel May 31, 2024 18:25
@BryanttV BryanttV force-pushed the bav/remove-unused-backends branch from 756bd81 to 1680555 Compare May 31, 2024 21:10
@github-actions github-actions bot added size/s and removed size/xs labels May 31, 2024
@BryanttV BryanttV requested a review from a team May 31, 2024 21:16
@BryanttV BryanttV force-pushed the bav/remove-unused-backends branch from 2c253ab to d8c8143 Compare June 4, 2024 17:39
@BryanttV BryanttV requested review from a team and removed request for a team June 4, 2024 18:12
@mariajgrimaldi
Copy link
Collaborator

We need to update the compatibility notes to remove the deleted backend in the ironwood notes: https://github.com/eduNEXT/eox-tenant/blob/master/README.rst#compatibility-notes. Also, could you improve the formatting in the compatibility notes table? Thanks!

@BryanttV
Copy link
Contributor Author

BryanttV commented Jun 4, 2024

Hi @mariajgrimaldi, thanks for the review.

We need to update the compatibility notes to remove the deleted backend in the ironwood notes

I also thought about doing it, however, @bra-i-am tells me it should not be removed because this backend does exist if we install a version of eox-tenant compatible with Ironwood, e.g. v2.6.0. If you check the other backends mentioned in the Ironwood notes, you can see that these files no longer exist but are still in README.

could you improve the formatting in the compatibility notes table?

Yes, I can format it correctly.

@mariajgrimaldi
Copy link
Collaborator

@BryanttV: good to know, thanks for the explanation!

Copy link
Contributor

@bra-i-am bra-i-am left a comment

Choose a reason for hiding this comment

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

LGTM!


We need to update the compatibility notes to remove the deleted backend in the ironwood notes

@mariajgrimaldi you are right, we need to update not only the compatibility notes but also the whole readme so it is more appropriate and trustable, nevertheless, taking into account that the readme at this moment works as @BryanttV mentioned, I believe we can wait for this card, but if you consider this needs to be done right now, there's no problem to me

@BryanttV
Copy link
Contributor Author

BryanttV commented Jun 5, 2024

Since another card already exists, I will merge with these changes.

@BryanttV BryanttV merged commit 3393187 into master Jun 5, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants