-
Notifications
You must be signed in to change notification settings - Fork 301
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
Programming exercises
: Support python exercises for local continuous integration
#7369
Conversation
…into feature/localci-python-support
Programming Exercises
: Support Python exercises for local continuous integrationProgramming exercises
: Support python exercises for local continuous integration
…into feature/localci-python-support
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.
Left some small comments for the code.
Also if this is not done yet: Since you have to add support for new programming languages at multiple places, it would make sense to document these places somewhere in the artemis docs for future reference by other devs.
.../java/de/tum/in/www1/artemis/service/connectors/localci/LocalCIBuildJobExecutionService.java
Outdated
Show resolved
Hide resolved
@@ -68,8 +69,8 @@ public class LocalCIBuildJobExecutionService { | |||
@Value("${artemis.version-control.url}") | |||
private URL localVCBaseUrl; | |||
|
|||
@Value("${artemis.version-control.local-vcs-repo-path}") | |||
private String localVCBasePath; | |||
@Value("${artemis.repo-clone-path}") |
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.
Please notify me once this PR was merged, so I can adjust TS3 and TS6 for this change
.../java/de/tum/in/www1/artemis/service/connectors/localci/LocalCIBuildJobExecutionService.java
Show resolved
Hide resolved
src/main/java/de/tum/in/www1/artemis/service/connectors/localci/LocalCIService.java
Show resolved
Hide resolved
.../java/de/tum/in/www1/artemis/service/connectors/localci/LocalCIBuildJobExecutionService.java
Show resolved
Hide resolved
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.
Tested in testing session. Python exercise compiled successfully.
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.
Tested in testing session. Py compiled.
333a367
Thank you for the code review @Hialus! I implemented your suggested changes. Concerning the documentation for future developers, I will do this in a follow up PR. |
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.
Reapprove, latest changes look good
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.
reapprove - lgtm
Checklist
General
Server
Changes affecting Programming Exercises
Motivation and Context
With this PR, the local CI system supports Python exercises.
Description
I added Python options for the build script generation. Test results get parsed accordingly.
Steps for Testing
Prerequisites:
Review Progress
Performance Review
Code Review
Manual Tests
Test Coverage
Screenshots