Skip to content

Remove ghost from the codebase #8363

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

Merged
merged 1 commit into from
Feb 28, 2022
Merged

Remove ghost from the codebase #8363

merged 1 commit into from
Feb 28, 2022

Conversation

aledbf
Copy link
Member

@aledbf aledbf commented Feb 21, 2022

Description

Now that ws-scheduler was removed and we are not using ghost workplaces anymore, remove it

How to test

Release Notes

Remove ghost from the codebase

Documentation

@aledbf aledbf requested review from a team February 21, 2022 20:43
@aledbf aledbf requested a review from csweichel as a code owner February 21, 2022 20:43
@github-actions github-actions bot added team: IDE team: devx team: webapp Issue belongs to the WebApp team team: workspace Issue belongs to the Workspace team labels Feb 21, 2022
@github-actions github-actions bot added team: IDE team: devx team: webapp Issue belongs to the WebApp team team: workspace Issue belongs to the Workspace team labels Feb 21, 2022
@aledbf aledbf force-pushed the aledbf/ghosts branch 2 times, most recently from 6fa4ce2 to 0b6a68b Compare February 21, 2022 23:14
@aledbf aledbf requested a review from a team February 21, 2022 23:14
@github-actions github-actions bot added the team: delivery Issue belongs to the self-hosted team label Feb 21, 2022
@codecov
Copy link

codecov bot commented Feb 21, 2022

Codecov Report

Merging #8363 (b4eec49) into main (d9b9f30) will increase coverage by 19.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##            main    #8363       +/-   ##
==========================================
+ Coverage   8.41%   27.42%   +19.00%     
==========================================
  Files         33       44       +11     
  Lines       2340     5733     +3393     
==========================================
+ Hits         197     1572     +1375     
- Misses      2138     4044     +1906     
- Partials       5      117      +112     
Flag Coverage Δ
components-gitpod-cli-app 11.17% <ø> (ø)
components-local-app-app-darwin-amd64 ?
components-local-app-app-darwin-arm64 ?
components-local-app-app-linux-amd64 ?
components-local-app-app-linux-arm64 ?
components-local-app-app-windows-386 ?
components-local-app-app-windows-amd64 ?
components-local-app-app-windows-arm64 ?
components-ws-manager-app 39.52% <0.00%> (?)
install-installer-raw-app 4.49% <ø> (-0.09%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
components/ws-manager/pkg/manager/create.go 82.95% <ø> (ø)
components/ws-manager/pkg/manager/monitor.go 9.46% <0.00%> (ø)
...l/installer/pkg/components/ws-manager/configmap.go 23.75% <ø> (-0.48%) ⬇️
components/local-app/pkg/auth/auth.go
components/local-app/pkg/auth/pkce.go
components/ws-manager/pkg/manager/probe.go 0.00% <0.00%> (ø)
...-manager/pkg/manager/internal/workpool/workpool.go 100.00% <0.00%> (ø)
components/ws-manager/pkg/manager/manager_ee.go 0.00% <0.00%> (ø)
components/ws-manager/pkg/clock/clock.go 68.62% <0.00%> (ø)
components/ws-manager/pkg/manager/imagespec.go 0.00% <0.00%> (ø)
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d9b9f30...b4eec49. Read the comment docs.

Copy link
Contributor

@mads-hartmann mads-hartmann left a comment

Choose a reason for hiding this comment

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

Awesome 🎉 I left two clarifying comments that I'd like to have addressed before I can approve ☺️

@@ -1,5 +1,5 @@
/**
* Copyright (c) 2021 Gitpod GmbH. All rights reserved.
* Copyright (c) 2022 Gitpod GmbH. All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

I was under the impression we shouldn't update these (ref)

Copy link
Member

@geropl geropl Feb 22, 2022

Choose a reason for hiding this comment

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

💯 ☝️ But this is a generated file, so not sure it is relevant. 🤷

@princerachit
Copy link
Contributor

Everything looks good, would like to get info on @mads-hartmann 's question and did we test this with helm ?

geropl
geropl previously approved these changes Feb 22, 2022
Copy link
Member

@geropl geropl left a comment

Choose a reason for hiding this comment

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

LGTM

mads-hartmann
mads-hartmann previously approved these changes Feb 22, 2022
Copy link
Contributor

@mads-hartmann mads-hartmann left a comment

Choose a reason for hiding this comment

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

LGTM 🚀 The only outstanding comment is about the copyright header update, but sounds like it's an autogenerated so might not be relevant (comment reference)

@aledbf aledbf dismissed stale reviews from mads-hartmann and geropl via 9316b22 February 22, 2022 15:48
mads-hartmann
mads-hartmann previously approved these changes Feb 23, 2022
Copy link
Contributor

@mads-hartmann mads-hartmann left a comment

Choose a reason for hiding this comment

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

Can't quite figure out what the diff is between now and the last review I gave, but scanning the code things still LGTM, so approving again.

@kylos101
Copy link
Contributor

@aledbf can we also remove from here?

Copy link
Contributor

@kylos101 kylos101 left a comment

Choose a reason for hiding this comment

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

LGTM

@mustard-mh
Copy link
Contributor

Should we clean these up too?

image

@roboquat roboquat merged commit 90fe82a into main Feb 28, 2022
@roboquat roboquat deleted the aledbf/ghosts branch February 28, 2022 08:47
@roboquat roboquat added the deployed: IDE IDE change is running in production label Mar 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: IDE IDE change is running in production release-note size/XXL team: delivery Issue belongs to the self-hosted team team: devx team: IDE team: webapp Issue belongs to the WebApp team team: workspace Issue belongs to the Workspace team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants