Skip to content

Update description for references.maxCachedProcesses#8468

Merged
bobbrow merged 3 commits intomainfrom
bobbrow/nullSetting
Dec 2, 2021
Merged

Update description for references.maxCachedProcesses#8468
bobbrow merged 3 commits intomainfrom
bobbrow/nullSetting

Conversation

@bobbrow
Copy link
Member

@bobbrow bobbrow commented Nov 30, 2021

Addresses insiders feedback in #4036

@sean-mcmanus
Copy link
Contributor

No, the 0 default is "by design". I believe it should be opt-in and not to use the null default.

Copy link
Contributor

@sean-mcmanus sean-mcmanus left a comment

Choose a reason for hiding this comment

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

...see my comment.

@bobbrow
Copy link
Member Author

bobbrow commented Nov 30, 2021

Hmm... We need to update the description then.

@bobbrow bobbrow changed the title Fix incorrect default value for references.maxCachedProcesses Update description for references.maxCachedProcesses Nov 30, 2021
@sean-mcmanus
Copy link
Contributor

Sure, I'm okay with the updated description -- but the default value was intentionally left out because it's not really necessary to describe the default since it shows in the UI...but we do for some settings, so I'm not sure what the best practice is in regards to that.

@bobbrow
Copy link
Member Author

bobbrow commented Nov 30, 2021

I think one of the problems here is that the other maxCachedProcesses settings do not disable the feature when you set it to 0. They have a min value of 2 internally, but this one does not. Setting it to 0 here turns it off.

@sean-mcmanus
Copy link
Contributor

I think one of the problems here is that the other maxCachedProcesses settings do not disable the feature when you set it to 0. They have a min value of 2 internally, but this one does not. Setting it to 0 here turns it off.

What's the problem? Yeah, I wasn't sure about whether the internal min value of 2 should be propogated to the TypeScript's min value or not.

@bobbrow bobbrow merged commit 1ebb6aa into main Dec 2, 2021
@bobbrow bobbrow deleted the bobbrow/nullSetting branch December 2, 2021 16:49
@bobbrow
Copy link
Member Author

bobbrow commented Dec 2, 2021

The problem is that the settings all have the same name, but not the same defaults, so I wanted to document it better.

@github-actions github-actions bot locked and limited conversation to collaborators Jan 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants