-
Notifications
You must be signed in to change notification settings - Fork 43
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: move is_docker_rootless from tutor to tutor discovery #93
chore: move is_docker_rootless from tutor to tutor discovery #93
Conversation
Faraz32123
commented
Nov 22, 2024
- move is_docker_rootless method from tutor to tutor-discovery
- move is_docker_rootless related tests from tutor to tutor-discovery and modify makefile according to it.
tutordiscovery/plugin.py
Outdated
@@ -95,7 +96,7 @@ | |||
|
|||
|
|||
# Automount /openedx/discovery folder from the container | |||
@tutor_hooks.Filters.COMPOSE_MOUNTS.add() | |||
@tutor_hooks.Filters.COMPOSE_MOUNTS.add() # type: ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, let's not type-ignore this (same below).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually this was needed to run pylint checks which were failing before, is there any other proposition. Or we can just remove it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your "ignore" statement is not for pylint but for mypy. Mypy checks are failing probably because tutor is installed in editable mode: overhangio/tutor#956
These tests should not fail in CI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ohk my bad, had to say mypy :)
If that's the case, I will remove it now.
@@ -0,0 +1,26 @@ | |||
import subprocess |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice job moving the unit tests here!
- move is_docker_rootless method from tutor to tutor-discovery - move is_docker_rootless related tests from tutor to tutor-discovery and modify makefile according to it.
ac52a96
to
bdb15ed
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
niiiice!