Skip to content

Remove the default value from idle-timeout #4756

Merged
reybard merged 1 commit intotrunkfrom
no-idle-default
Nov 19, 2021
Merged

Remove the default value from idle-timeout #4756
reybard merged 1 commit intotrunkfrom
no-idle-default

Conversation

@reybard
Copy link
Contributor

@reybard reybard commented Nov 18, 2021

#4741 added idle-timeout as a param that could be specified on codespace creation. This works well for those overrides, but users can also set a default timeout in the UI in their codespace settings. This user setting should take precedence over the default 30 minute timeout that happens when the user has no specific value set.

With the way it works now, the default value on the new param means that not specifying any idle timeout will always use 30 minutes, which could be very different to the value the user has saved.

The order of precendence is GH CLI override > codespace settings UI override > VSCS default (30 mins).

This change removes the default and omits the value entirely from the API call when not set, which properly falls back to the user's saved setting if available and the 30 min default if the setting is not saved for the user.

@reybard reybard requested a review from a team as a code owner November 18, 2021 19:42
@cliAutomation
Copy link
Collaborator

Hi! Thanks for the pull request. Please ensure that this change is linked to an issue by mentioning an issue number in the description of the pull request. If this pull request would close the issue, please put the word 'Fixes' before the issue number somewhere in the pull request body. If this is a tiny change like fixing a typo, feel free to ignore this message.

@reybard reybard merged commit 3aef78f into trunk Nov 19, 2021
@reybard reybard deleted the no-idle-default branch November 19, 2021 23:01
@VictorBatta VictorBatta mentioned this pull request Dec 4, 2021
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