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.

Sign up for GitHub

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

Jump to bottom

Development: Decouple LocalCI and LocalVC #7787

Merged
merged 18 commits into from
Dec 17, 2023
Merged

Development: Decouple LocalCI and LocalVC #7787

merged 18 commits into from
Dec 17, 2023

Conversation

mateusmm01
Copy link
Contributor

@mateusmm01 mateusmm01 commented Dec 12, 2023 •
edited by krusche
Loading

Checklist

General

Server

Motivation and Context

Currently, the LocalVC and LocalCI are highly coupled inside of the LocalCIConnectorService. This was causing issues for further development.

Description

The code was moved to the LocalVCServletService. Instead of directly using the LocalCITriggerService we now you use the interface ContinuousIntegrationTriggerService for triggering the builds.

Steps for Testing

Prerequisites:

  • 1 Instructor

Create a few programming exercises and submit a few changes and check that the system still works as expected

Testserver States

Note

These badges show the state of the test servers.
Green = Currently available, Red = Currently locked







Review Progress

Performance Review

  • I (as a reviewer) confirm that the server changes (in particular related to database calls) are implemented with a very good performance

Code Review

  • Code Review 1
  • Code Review 2

Manual Tests

  • Test 1
  • Test 2

@mateusmm01 mateusmm01 changed the title Enhancement/localci localvc/decoupling of localci and localvc Enhancement: Decouple LocalCI and LocalVC by refactoring old LocalCIConnectorService Dec 12, 2023
@mateusmm01 mateusmm01 marked this pull request as ready for review December 14, 2023 11:43
@mateusmm01 mateusmm01 requested a review from a team as a code owner December 14, 2023 11:43
@farisd16 farisd16 temporarily deployed to artemis-test2.artemis.cit.tum.de December 14, 2023 15:34 — with GitHub Actions Inactive
@farisd16 farisd16 temporarily deployed to artemis-test2.artemis.cit.tum.de December 14, 2023 19:10 — with GitHub Actions Inactive
jakubriegel
jakubriegel approved these changes Dec 16, 2023
View reviewed changes
Copy link
Contributor

@jakubriegel jakubriegel left a comment

Choose a reason for hiding this comment

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

Code ok after update

laurenzfb
laurenzfb approved these changes Dec 17, 2023
View reviewed changes
Copy link
Contributor

@laurenzfb laurenzfb left a comment •
edited
Loading

Choose a reason for hiding this comment

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

code lgtm. also tested on TS3, did not encounter any issues. Nicely done :)

@github-actions GitHub Actions
Copy link

⚠️ Unable to deploy to test servers ⚠️

Testserver "artemis-test3.artemis.cit.tum.de" is already in use by PR #7791.

@github-actions github-actions bot added the deployment-error Added by deployment workflows if an error occured label Dec 17, 2023
@laurenzfb laurenzfb added deploy:artemis-test3 and removed deployment-error Added by deployment workflows if an error occured labels Dec 17, 2023
@laurenzfb laurenzfb temporarily deployed to artemis-test3.artemis.cit.tum.de December 17, 2023 14:36 — with GitHub Actions Inactive
@github-actions github-actions bot mentioned this pull request Dec 17, 2023
Merged
11 tasks
@valentin-boehm valentin-boehm temporarily deployed to artemis-test6.artemis.cit.tum.de December 17, 2023 15:09 — with GitHub Actions Inactive
valentin-boehm
valentin-boehm approved these changes Dec 17, 2023
View reviewed changes
Copy link
Contributor

@valentin-boehm valentin-boehm left a comment

Choose a reason for hiding this comment

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

Tested on TS6, did not find any issues

@krusche krusche changed the title Enhancement: Decouple LocalCI and LocalVC by refactoring old LocalCIConnectorService Development: Decouple LocalCI and LocalVC Dec 17, 2023
@krusche krusche added this to the 6.7.3 milestone Dec 17, 2023
@krusche krusche merged commit f9198cd into develop Dec 17, 2023
92 of 98 checks passed
@krusche krusche deleted the enhancement/localci-localvc/decoupling-of-localci-and-localvc branch December 17, 2023 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jakubriegel jakubriegel jakubriegel approved these changes

@laurenzfb laurenzfb laurenzfb approved these changes

@Ceiridge Ceiridge Ceiridge approved these changes

@valentin-boehm valentin-boehm valentin-boehm approved these changes

Assignees

@mateusmm01 mateusmm01

Labels
component:integrated code lifecycle ready to merge
Projects
None yet
Milestone
6.7.3
Development

Successfully merging this pull request may close these issues.
8 participants
@mateusmm01 @jakubriegel @laurenzfb @Ceiridge @valentin-boehm @krusche @farisd16 @Mtze