Skip to content

Add IDE versions to the dashboard #15139

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 2 commits into from
Dec 6, 2022
Merged

Conversation

filiptronicek
Copy link
Member

@filiptronicek filiptronicek commented Dec 3, 2022

Description

This PR is simply adding versions below the icons of all IDEs in the IDE switcher component (used during on-boarding and always available in the user preferences).

This is partially a lie because we don't associate VS Code Desktop with a version, but it holds true for the rest.
image

It's a relatively easy win because we:

  1. Already tag all newly-built IDE images with their version strings (Label JetBrains IDE images with their versions #15082, Tag VS Code images with version #14055)
  2. We already implemented the functionality in ide-service so that it returns the IDE versions along the IDE options (Add IDE versions to ide service #15081)

With this we can do two things at once:

  • keep our users informed what versions we use right from the Dashboard (good for checking new versions with bug fixes and features)
  • makes it easier for us to check that our IDEs are at their correct versions when releasing them at IDE (the team).

How to test

  1. Go to the preview environment and sign in
  2. You should be greeted with a modal with the IDEs and their versions, you can also visit the user prefs to see it.

Release Notes

Add versions of all the supported IDEs to the Preferences page

Documentation

Werft options:

  • /werft with-local-preview
    If enabled this will build install/preview
  • /werft with-preview
  • /werft with-large-vm
  • /werft with-integration-tests=all
    Valid options are all, workspace, webapp, ide, jetbrains, vscode, ssh

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-ft-add-ide-versions-dashboard.1 because the annotations in the pull request description changed
(with .werft/ from main)

@filiptronicek filiptronicek force-pushed the ft/add-ide-versions-dashboard branch from 5382413 to fffef66 Compare December 3, 2022 12:14
@filiptronicek filiptronicek force-pushed the ft/add-ide-versions-dashboard branch from fffef66 to c5a4f9c Compare December 3, 2022 12:28
@filiptronicek filiptronicek marked this pull request as ready for review December 3, 2022 12:51
@filiptronicek filiptronicek requested a review from a team December 3, 2022 12:51
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Dec 3, 2022
function renderIdeOption(
option: IDEOption,
selected: boolean,
version: IDEOption["imageVersion"],
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't the version already be inside the option: IDEOption param?

Copy link
Member

@akosyakov akosyakov Dec 5, 2022

Choose a reason for hiding this comment

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

option has 2 versions stable and latest, passed version is a selection one of another depending on a user choice (line 98 above)

Comment on lines 79 to 81
// TODO: Compatible with ide-config not deployed, need revert after ide-config deployed
delete ideopts.options["code-latest"];
delete ideopts.options["code-desktop-insiders"];
delete ideOptions.options["code-latest"];
delete ideOptions.options["code-desktop-insiders"];
Copy link
Member

Choose a reason for hiding this comment

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

What does this relate to? Can we remove this logic?

Copy link
Member

Choose a reason for hiding this comment

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

yes, it can be removed already cc @mustard-mh

@akosyakov
Copy link
Member

akosyakov commented Dec 5, 2022

@filiptronicek very nice job ⭐

@mustard-mh Could you test please it to bring it in, looking forward! 🚀

@gtsiolis
Copy link
Contributor

gtsiolis commented Dec 5, 2022

/werft run

👍 started the job as gitpod-build-ft-add-ide-versions-dashboard.7
(with .werft/ from main)

Copy link
Member

@easyCZ easyCZ left a comment

Choose a reason for hiding this comment

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

LGTM

/hold for you to merge when ready

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.

Seems like this breaks the dashboard UX. ❗

Modal (Signing up) Preferences (User Settings)
Screenshot 2022-12-05 at 1 43 56 PM (2) Screenshot 2022-12-05 at 1 45 10 PM (2)

@gtsiolis
Copy link
Contributor

gtsiolis commented Dec 5, 2022

Regarding the comment above in #15139 (review), force reloading helped, not sure if something didn't load ok on the first run. 🤷 Cc @filiptronicek

function renderIdeOption(
option: IDEOption,
selected: boolean,
version: IDEOption["imageVersion"],
Copy link
Contributor

Choose a reason for hiding this comment

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

question: When selecting the latest release, VS Code version remains the same. Is this expected?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think here it's because in preview environments, we just use the stable version as Insiders. Should be expected

@iQQBot please correct me if I'm wrong here 😄 🙏

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, @filiptronicek! OK, let's see how this behaves in production, then. 🎣

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.

Leaving one last comment, following up from a relevant discussion (internal), it should be ok to merge this and create follow-up issues or PRs for the outstanding design details.

Looping in @filiptronicek, @mustard-mh, and @akosyakov to release the hold break if this sounds ok. 🏓

Cross-posting for visibility:

Maybe using a different label variant to not associate these two kinds of information.

editors

Copy link
Contributor

@mustard-mh mustard-mh left a comment

Choose a reason for hiding this comment

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

Code, UX LGTM.

Tested with preview env, worked like a charm 🎉

Seems like this breaks the dashboard UX. ❗

@gtsiolis Maybe related to network issue? you can check your DevTool Console and Network tabs next time you faced it. 🙏

ide-service will need to fetch and process image information when it first start, during this time, server will not get data. So it can happen also when server has no cache for it yet, and ide-service is starting. But it should not be your case, as no one is going to restart it.

We need to handle healthy check in ide-service properly.

@akosyakov
Copy link
Member

Can be unhold?

@filiptronicek
Copy link
Member Author

/unhold

@gtsiolis
Copy link
Contributor

gtsiolis commented Dec 6, 2022

@filiptronicek Did you want me to re-review the UX changes in #15139 (comment)? Seems like not much changed from UX perspective.

@akosyakov
Copy link
Member

@gtsiolis I guess you need approve to unblock

Not sure why it is not mergeable

@roboquat roboquat merged commit 0f97962 into main Dec 6, 2022
@roboquat roboquat deleted the ft/add-ide-versions-dashboard branch December 6, 2022 16:53
@filiptronicek
Copy link
Member Author

filiptronicek commented Dec 6, 2022

Sorry for the confusion, @gtsiolis 🙏. The re-request was triggered because I thought it would unblock merging, it seems that just dismissing your review did the trick (thanks Anton!).

@gtsiolis
Copy link
Contributor

gtsiolis commented Dec 6, 2022

Thanks @filiptronicek & @akosyakov!

Opened follow-up issue #15194 to keep track of the design nitpicks based on our discussion (internal).

@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: dashboard deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note size/M team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants