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

Remove Theia settings and point to Code image #5511

Merged
merged 1 commit into from
Sep 3, 2021
Merged

Remove Theia settings and point to Code image #5511

merged 1 commit into from
Sep 3, 2021

Conversation

laushinka
Copy link
Contributor

@laushinka laushinka commented Sep 2, 2021 •
edited
Loading

This PR:

  1. Removes the Theia option in Preferences
  2. Points the Theia alias to the Code stable image.

/werft with-clean-slate-deployment

Fixes #4486

@roboquat roboquat added the size/M label Sep 2, 2021
@laushinka
Copy link
Contributor Author

laushinka commented Sep 2, 2021 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-laushinka-remove-theia-option-in-4486.10

@gtsiolis
Copy link
Contributor

gtsiolis commented Sep 3, 2021 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-laushinka-remove-theia-option-in-4486.11

@gtsiolis
Copy link
Contributor

gtsiolis commented Sep 3, 2021 •
edited by werft-gitpod-dev-com bot
Loading

Looking at this now! 👀

gtsiolis
gtsiolis approved these changes Sep 3, 2021
View reviewed changes
Copy link
Contributor

@gtsiolis gtsiolis left a comment

Choose a reason for hiding this comment

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

Thanks for making this change @laushinka! 🌟

UX looks good for new users. Wondering how we could test this for existing users who previously had Theia as the selected editor. FWIW, I could test this also in staging with a couple of test users.

chart/templates/server-deployment.yaml
@@ -161,9 +161,9 @@ spec:
- name: THEIA_PORT
value: "{{ .Values.components.workspace.ports.http.containerPort | toString }}"
- name: THEIA_IMAGE_REPO
Copy link
Contributor

Choose a reason for hiding this comment

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

question: Shall we open a follow up issue to remove this later?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we should create a follow-up issue to clean up the code if we don't have it yet.

components/dashboard/src/settings/Preferences.tsx
Comment on lines -53 to -55
<AlertBox className="mt-3 mb-4 w-3/4">
We're deprecating the Theia editor. You can still switch back to Theia for the next few weeks but the preference will be removed by the end of August 2021.
</AlertBox>
Copy link
Contributor

Choose a reason for hiding this comment

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

praise: Thanks for also removing this, @laushinka! 🎗️

@roboquat roboquat added the lgtm label Sep 3, 2021
@laushinka
Copy link
Contributor Author

laushinka commented Sep 3, 2021 •
edited
Loading

UX looks good for new users. Wondering how we could test this for existing users who previously had Theia as the selected editor. FWIW, I could test this also in staging with a couple of test users.

@gtsiolis I tested it yesterday by setting the DB for the default IDE for my user to 'theia' (in my preview env), and then opening up a workspace. I can show you quickly if you're able to access the staging DB.

@laushinka laushinka marked this pull request as ready for review September 3, 2021 08:05
@roboquat roboquat removed the lgtm label Sep 3, 2021
@laushinka laushinka force-pushed the laushinka/remove-theia-option-in-4486 branch from 98f9d3f to 3cb70fe Compare September 3, 2021 11:57
laushinka
laushinka commented Sep 3, 2021
View reviewed changes
Copy link
Contributor Author

@laushinka laushinka left a comment

Choose a reason for hiding this comment

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

When the user selects code we don't have to delete the settings actually because the default is already 'code'. We can see here

gitpod/components/gitpod-db/src/typeorm/user-db-impl.ts

Line 81 in f66598c

ideSettings: { defaultIde: 'code' },

@JanKoehnlein JanKoehnlein force-pushed the laushinka/remove-theia-option-in-4486 branch 2 times, most recently from 3cb70fe to 294af41 Compare September 3, 2021 15:31
@JanKoehnlein JanKoehnlein self-requested a review September 3, 2021 15:32
@JanKoehnlein
Copy link
Contributor

/lgtm

@roboquat roboquat added the lgtm label Sep 3, 2021
@roboquat
Copy link
Contributor

roboquat commented Sep 3, 2021

LGTM label has been added.

Git tree hash: 4d1118cd041a1a6363a420607fac6f3f48231fba

JanKoehnlein
JanKoehnlein approved these changes Sep 3, 2021
View reviewed changes
Copy link
Contributor

@JanKoehnlein JanKoehnlein left a comment

Choose a reason for hiding this comment

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

LGTM

@roboquat
Copy link
Contributor

roboquat commented Sep 3, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gtsiolis, JanKoehnlein

Associated issue: #4486

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@roboquat roboquat merged commit 7ed6fc0 into main Sep 3, 2021
@roboquat roboquat deleted the laushinka/remove-theia-option-in-4486 branch September 3, 2021 15:42
@gtsiolis
Copy link
Contributor

gtsiolis commented Sep 3, 2021 •
edited by werft-gitpod-dev-com bot
Loading

IT IS DONE! ☄️

@filiptronicek filiptronicek mentioned this pull request Jan 7, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JanKoehnlein JanKoehnlein JanKoehnlein approved these changes

@gtsiolis gtsiolis gtsiolis approved these changes

Assignees

@gtsiolis gtsiolis

@JanKoehnlein JanKoehnlein

Labels
approved size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove theia option in settings
4 participants
@laushinka @gtsiolis @JanKoehnlein @roboquat