Skip to content

Use ports order from supervisor #444

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
Oct 19, 2022
Merged

Use ports order from supervisor #444

merged 1 commit into from
Oct 19, 2022

Conversation

mustard-mh
Copy link

@mustard-mh mustard-mh commented Oct 17, 2022

Ports order is random in vscode now, because we don't clean up map in portViewProvider.ts, this PR will make order respect to supervisor's responding

How to test

Check with gitpod-io/gitpod#13788

@akosyakov
Copy link
Member

Is not Map in JS have insertion order? 🤔

@mustard-mh
Copy link
Author

Is not Map in JS have insertion order? 🤔

@akosyakov According to its define (just know it), it will. But inside portViewProvider.ts we don't clear map and recreate it.

Found it because order not align to response's order in preview env

So it makes sense to keep code inside portViewProvider.ts only? and remove extension.ts's changes?

@akosyakov
Copy link
Member

@akosyakov According to its define (just know it), it will. But inside portViewProvider.ts we don't clear map and recreate it.

ok, make sense 👍

@akosyakov akosyakov force-pushed the gp-code/main branch 2 times, most recently from bda0ea6 to b724dee Compare October 18, 2022 23:09
Copy link

@iQQBot iQQBot left a comment

Choose a reason for hiding this comment

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

Test it in gitpod-io/gitpod#13788 🎉

@mustard-mh mustard-mh merged commit c5ab773 into gp-code/main Oct 19, 2022
@iQQBot iQQBot deleted the hw/port-order branch February 10, 2023 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants