Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
feat: coder connect integration #482
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
feat: coder connect integration #482
Changes from 1 commit
95b5b4e
a65e550
2ecf1df
fb9a263
3a77138
2a3500e
9252fff
195151a
e7cad82
ea4b179
5e4e795
c3287eb
a2df5cc
feb1021
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The extension has an SSH config section which I assume is not going apply to Coder Connect, is that right? As well as anything the user has added to their SSH config manually.
Wondering if we should update the setting description to say the variables will not apply if there is a Coder Connect session we can use.
Speaking of settings, should using Coder Connect be behind a setting as well? Or a prompt? "We see Coder Connect, do you want to use that?" or similar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will add.
These will apply, as long as they're on the Coder Connect hostname (so, if they just specify those options when running
coder config-ssh
it'll get added to*.<hostnamesuffix>
).I don't think this is necessary, I can't imagine a scenario where the user would have Coder Connect on and not want to use it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the tricky part is that the user might not realize it will use Connect. So, they have
X11Forwarding yes
in their VS Code config or in their SSH config underHost coder-vscode--*
, they are trying out Connect, one day they update the plugin, and suddenly no more forwarding. Some days it does forward, some days does not, depending on whether Connect is up (idk if this is realistic usage/behavior though).So there should be clear messaging what the plugin is going to do IMO, somewhere. Silently changing behavior feels bad to me. But, I am neither a VS Code nor Connect user, so take what I say with a grain of salt. 😆 Maybe the behavior change will be obvious to users.
If we could make the same SSH settings apply either way, then that would be a different story.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the updated
coder ssh-config
, any options applied to that will also apply to Coder Connect via VSCode, at least.Though if we change the approach to what I discussed below that solves this issue too ig
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is quite nice!