Skip to content

Hide the latest ide alert warning on workspace timeout #16551

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
Mar 21, 2023

Conversation

Devansu-Yadav
Copy link
Contributor

@Devansu-Yadav Devansu-Yadav commented Feb 24, 2023

Description

  • Removes the latest ide alert warning on the workspace timeout page and only shows it during workspace (re)start.

Related Issue(s)

Relates #11035

How to test

  • Open workspace in preview env with a latest editor https://hide-editor-alert.preview.gitpod-dev.com/workspaces
  • Alert should show during starting workspace
  • Stop to see if the latest ide alert does not show up
  • Open again with a stable editor
  • Alert should not show during starting workspace
  • Stop should have no alert

Release Notes

Removes the latest ide alert warning on the workspace timeout page

Documentation

Build Options:

  • /werft with-github-actions
    Experimental feature to run the build with GitHub Actions (and not in Werft).
  • /werft with-werft
  • leeway-no-cache
    leeway-target=components:all
  • /werft no-test
    Run Leeway with --dont-test
Publish Options
  • /werft publish-to-npm
  • /werft publish-to-jb-marketplace
Installer Options
  • with-ee-license
  • with-slow-database
  • with-dedicated-emulation
  • with-ws-manager-mk2
  • workspace-feature-flags
    Add desired feature flags to the end of the line above, space separated

Preview Environment Options:

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

@Devansu-Yadav Devansu-Yadav requested a review from a team February 24, 2023 09:10
@werft-gitpod-dev-com
Copy link

annotations in the pull request changed, but user is not allowed to start a job

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-hide-editor-alert.0 because the annotations in the pull request description changed
(with .werft/ from main)

@gtsiolis
Copy link
Contributor

gtsiolis commented Feb 24, 2023

/werft run with-preview=true

👍 started the job as gitpod-build-hide-editor-alert-fork.0
(with .werft/ from main)

@Devansu-Yadav
Copy link
Contributor Author

Devansu-Yadav commented Feb 28, 2023

/werft run with-preview=true

👍 started the job as gitpod-build-hide-editor-alert-fork.0 (with .werft/ from main)

@gtsiolis Do the changes in this PR hide the warning on the workspace timeout page? I can't seem to test this change as I don't have permission to create a workspace in the preview environment 😅

@gtsiolis

This comment was marked as outdated.

@gtsiolis

This comment was marked as outdated.

@gtsiolis
Copy link
Contributor

gtsiolis commented Mar 2, 2023

/werft run with-preview=true

👍 started the job as gitpod-build-hide-editor-alert-fork.1
(with .werft/ from main)

@gtsiolis gtsiolis force-pushed the hide-editor-alert branch from 3618253 to 354dc2b Compare March 3, 2023 11:26
@gtsiolis
Copy link
Contributor

gtsiolis commented Mar 3, 2023

/werft run with-preview=true

👍 started the job as gitpod-build-hide-editor-alert-fork.2
(with .werft/ from main)

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.

UX LGTM, @Devansu-Yadav! 🌮 🌮

I could verify the warning is no longer present after the workspace timed out. 🏁

Approving to unblock merging but holding to pass this to @gitpod-io/engineering-webapp to pull the trigger in case there's anything that looks unnecessary and needs a revert or a different approach. 🏓

Looping in also @mustard-mh who recently edited this file to add this warning in the first place.

/hold

@Devansu-Yadav
Copy link
Contributor Author

Devansu-Yadav commented Mar 8, 2023

Approving to unblock merging but holding to pass this to @gitpod-io/engineering-webapp to pull the trigger in case there's anything that looks unnecessary and needs a revert or a different approach. 🏓

Looping in also @mustard-mh who recently edited this file to add this warning in the first place.

/hold

@gtsiolis Any updates on whether any changes are required in this PR or not?

@mustard-mh
Copy link
Contributor

mustard-mh commented Mar 8, 2023

/werft run

👍 started the job as gitpod-build-hide-editor-alert-fork.3
(with .werft/ from main)

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-hide-editor-alert.1 because the annotations in the pull request description changed
(with .werft/ from main)

@mustard-mh
Copy link
Contributor

mustard-mh commented Mar 8, 2023

Any updates on whether any changes are required in this PR or not?

Hey @Devansu-Yadav 👋 Thanks for your contributions!! 🧡

I prefer to call the variable isTimedOut and check with showLatestIdeWarning = !isTimedOut && useLatest

We will need review from @gitpod-io/engineering-webapp as codeowner

pinged in our development pipeline to resolve the preview env problem 🙏

@Devansu-Yadav
Copy link
Contributor Author

Devansu-Yadav commented Mar 9, 2023

I prefer to call the variable isTimedOut and check with showLatestIdeWarning = !isTimedOut && useLatest

@mustard-mh That seems much better! Should I go ahead and make this change?

We will need review from @gitpod-io/engineering-webapp as codeowner

Sure, I'll wait 😄

@mustard-mh
Copy link
Contributor

mustard-mh commented Mar 9, 2023

Should I go ahead and make this change?

@Devansu-Yadav Sure, let's go with it 🚀

After your changes, I will do the final test in preview env, and then unhold this PR since you get approve from WebApp 👍

@gtsiolis
Copy link
Contributor

gtsiolis commented Mar 9, 2023

/werft run with-preview

👍 started the job as gitpod-build-hide-editor-alert-fork.4
(with .werft/ from main)

@mustard-mh
Copy link
Contributor

mustard-mh commented Mar 9, 2023

I tested it in preview env, and it worked with Timed Out

But when with phases like Stopped Stopping, we still got this warning. I will unhold this PR and change (changed) the related issue's description for them. @Devansu-Yadav Thank you for your contribution! 🧡

Stopped Timed Out
image image

@mustard-mh
Copy link
Contributor

/unhold

@gtsiolis
Copy link
Contributor

gtsiolis commented Mar 9, 2023

@mustard-mh This can't be merged because the build fails after the new switch to GH Actions, right?

See relevant discussion (internal). Cc @vulkoingim

@Devansu-Yadav
Copy link
Contributor Author

Devansu-Yadav commented Mar 9, 2023

But when with phases like Stopped Stopping, we still got this warning. I will updated unhold this PR and change the related issue's description for them. @Devansu-Yadav Thank you for your contribution! 🧡

Ohh okay, but I guess the scope of this issue mostly focussed on the UX where we don't want to show this warning on the Timed Out phase, isn't it @gtsiolis? 😅

If needed I could probably address this either in this same PR or maybe open up a separate PR for them..

@mustard-mh
Copy link
Contributor

mustard-mh commented Mar 9, 2023

But when with phases like Stopped Stopping, we still got this warning. I will updated unhold this PR and change the related issue's description for them. @Devansu-Yadav Thank you for your contribution! 🧡

Ohh okay, but I guess the scope of this issue mostly focussed on the UX where we don't want to show this warning on the Timed Out phase, isn't it @gtsiolis? 😅

If needed I could probably address this either in this same PR or maybe open up a separate PR for them..

@Devansu-Yadav Yep, we can do it separately 👍 , updated the issue description you can pick it up if you want and get time 🙏

@Devansu-Yadav
Copy link
Contributor Author

Devansu-Yadav commented Mar 10, 2023

@Devansu-Yadav Yep, we can do it separately 👍 , updated the issue description you can pick it up if you want and get time 🙏

Great, I'll raise a PR for both these phases once this PR is merged 😄

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-hide-editor-alert.4 because the annotations in the pull request description changed
(with .werft/ from main)

@gtsiolis
Copy link
Contributor

gtsiolis commented Mar 10, 2023

Build seems successful. Removing hold.

Thanks @mustard-mh & @iQQBot for triggering the builds[1][2].

/unhold

@iQQBot
Copy link
Contributor

iQQBot commented Mar 10, 2023

It's looks not success, because
image

github action build is required, but this action will fail maybe because we don't send secret to fork, screenshot from https://github.blog/2020-08-03-github-actions-improvements-for-fork-and-pull-request-workflows/

image

@iQQBot
Copy link
Contributor

iQQBot commented Mar 10, 2023

/hold

cc @mads-hartmann

@gtsiolis
Copy link
Contributor

gtsiolis commented Mar 10, 2023

For visibility, I'm trying re-triggering the job with what @vulkoingim suggested[1] (internal) which helped merge another community contribution in #16417.

https://werft.gitpod-dev.com/job/gitpod-build-hide-editor-alert.6

@vulkoingim
Copy link
Contributor

Apologies for the back-and-forth on this issue @Devansu-Yadav . Currently we're doing changes around our build/CI systems and there are still some details we haven't smoothed out. I'll get back to you next week, once we have resolved the outstanding issues and will push this through.

@Devansu-Yadav
Copy link
Contributor Author

Devansu-Yadav commented Mar 18, 2023

Currently we're doing changes around our build/CI systems and there are still some details we haven't smoothed out. I'll get back to you next week, once we have resolved the outstanding issues and will push this through.

@vulkoingim Any updates on this? 😅

@vulkoingim vulkoingim merged commit 16b55bb into gitpod-io:main Mar 21, 2023
@vulkoingim
Copy link
Contributor

vulkoingim commented Mar 21, 2023

Currently we're doing changes around our build/CI systems and there are still some details we haven't smoothed out. I'll get back to you next week, once we have resolved the outstanding issues and will push this through.

@vulkoingim Any updates on this? 😅

Apologies for the delay - got it merged just now. Thank you for your contribution! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants