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 bok-choy #1165

Merged
merged 5 commits into from
Nov 20, 2023
Merged

Conversation

salman2013
Copy link
Contributor

Description:

As the bok-choy has been deprecated openedx/public-engineering#13, we are removing its usage from the code in this PR.

Ticket : https://github.com/orgs/openedx/projects/55/views/1?pane=issue&itemId=43871891

Copy link
Contributor

@feanil feanil left a comment

Choose a reason for hiding this comment

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

We should also drop the dependency on selenium but then I think this is good to merge.

# via requests
# via
# requests
# selenium
Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't look like anything else is using selenium either, so it's probably safe to remove that from test.in as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@feanil should i remove the selenium under the scope of this PR? or create separate one?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should remove selenium along with bok-choy if it's not being used independently of bok-choy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@feanil done.

@UsamaSadiq
Copy link
Member

Starting with tox 4.0 we need to specifically allow all scripts/tools to be used in the tox commands section.
In this specific case, we are directly mentioning the script which has not been specified in the allowlist_externals section for the version_check env.
To make it work with tox 4 (Tested on my end locally), we can simply replace the following line

{toxinidir}/edx_proctoring/scripts/version_check.py

with

python {toxinidir}/edx_proctoring/scripts/version_check.py

@feanil feanil force-pushed the salman/remove-bok-choy-usage branch from 50d01c0 to 5f446f2 Compare November 20, 2023 15:41
@feanil feanil merged commit 52dc8b6 into openedx:master Nov 20, 2023
16 checks passed
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.

4 participants