-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Make the new options module fully type safe #13620
base: master
Are you sure you want to change the base?
Changes from 1 commit
f42f773
13a4b19
c89c599
0d4ed70
aee5c19
9c48b1c
1127964
b6425df
58c4327
4a69ef0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -63,7 +63,8 @@ def get_options(self) -> coredata.MutableKeyedOptionDictType: | |
opts[key] = options.UserIntegerOption( | ||
self.make_option_name(key), | ||
'Number of threads to use in web assembly, set to 0 to disable', | ||
(0, None, 4)) # Default was picked at random | ||
4, # Default was picked at random | ||
min_value=0) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this may be a good case where it's worth explaining the change in the commit message. After thinking about this, I think that this works because the dataclass also fits a historic mess where the "value" member isn't consistently used by all option types, when we do stuff like overloading it with minimum/maximum values instead? |
||
|
||
return opts | ||
|
||
|
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.
nitpick: this one ends with a
.
, while other options don't...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 has apparently always existed. The main branch right now has that peroid