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

Quick fix to support changing what version of python to use #506

Closed
wants to merge 1 commit into from

Conversation

wil93
Copy link
Member

@wil93 wil93 commented Dec 13, 2015

I know that we should look into making compilers "separate" from CMS (with a plugin system). In the meantime, though, I think it's good to have a simple way to switch the python version used when evaluating python submissions.

Review on Reviewable

@stefano-maggiolo
Copy link
Member

Thanks for doing this, indeed I don't think we'll get around to change to a plugin system before 1.3. Do you see a good place where to document this new feature?


Reviewed 2 of 2 files at r1.
Review status: all files reviewed at latest revision, 2 unresolved discussions.


cms/grading/init.py, line 250 [r1] (raw file):
Can we not depend on the specific string "cpython-34", which I assume means python 3.4? https://www.python.org/dev/peps/pep-3147/#proposal suggests that that string identifies the version and I think it can be treated as an opaque id.


cms/grading/init.py, line 254 [r1] (raw file):
It would be cleaner to extract the basename and apply it immediately to both pyc_name


Comments from the review on Reviewable.io

@wil93 wil93 force-pushed the quick_py3_fix branch 2 times, most recently from be56f60 to a6136c9 Compare December 15, 2015 16:37
@wil93
Copy link
Member Author

wil93 commented Dec 15, 2015

We could put it in cms.conf.sample, or just hint at the existence of that option here http://cms.readthedocs.org/en/v1.2/Configuring%20a%20contest.html#programming-languages

(In the meantime I've done both)


Review status: 1 of 4 files reviewed at latest revision, 2 unresolved discussions.


cms/grading/init.py, line 250 [r1] (raw file):
I tried to use * but it doesn't work... so I thought it was enough to use the current version (for ubuntu 14.04)


cms/grading/init.py, line 254 [r1] (raw file):
Done.


Comments from the review on Reviewable.io

@stefano-maggiolo
Copy link
Member

Honestly I would revert it from cms.conf.sample - it is unlikely that a random user would use it and on the other hand it might be confusing.


Reviewed 3 of 3 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


cms/grading/init.py, line 250 [r1] (raw file):
I think you can inspect the content of the directory and find out the right name :)


Comments from the review on Reviewable.io

@wil93
Copy link
Member Author

wil93 commented Dec 15, 2015

Done.


Review status: all files reviewed at latest revision, 1 unresolved discussion.


cms/grading/init.py, line 250 [r1] (raw file):
Mmmm actually I'm not sure that the directory even exists at the time of executing that line of code 😕


Comments from the review on Reviewable.io

@stefano-maggiolo
Copy link
Member

This will be superseded by the language plugins that are in the works.

@wil93 wil93 deleted the quick_py3_fix branch February 24, 2016 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants