Document naming convention for boolean API fields

[pmorie]

Document to avoid IsBla-type names for boolean properties.

This is extremely consistent throughout our APIs, but not written down.

[duglin]

Can you add an explanation?

[eparis]

@kubernetes/api-approvers

[liggitt]

Any chance of an automated test to go along with this doc?

[erictune]

What are some examples? I tried to find some using this command:

$ grep "able.*bool" -R pkg/apis | grep types.go
pkg/apis/componentconfig/types.go:	EnableServer bool
pkg/apis/componentconfig/types.go:	EnableDebuggingHandlers bool
pkg/apis/componentconfig/types.go:	EnableContentionProfiling bool
pkg/apis/componentconfig/types.go:	RegisterSchedulable bool
pkg/apis/componentconfig/types.go:	EnableCustomMetrics bool
pkg/apis/componentconfig/types.go:	EnableControllerAttachDetach bool
pkg/apis/componentconfig/types.go:	MakeIPTablesUtilChains bool
pkg/apis/componentconfig/types.go:	EnableCRI bool
pkg/apis/componentconfig/types.go:	DockerEnableSharedPID bool
pkg/apis/componentconfig/types.go:	ExperimentalNodeAllocatableIgnoreEvictionThreshold bool
pkg/apis/componentconfig/types.go:	Enabled bool
pkg/apis/componentconfig/types.go:	Enabled bool
pkg/apis/componentconfig/types.go:	EnableProfiling bool
pkg/apis/componentconfig/types.go:	EnableContentionProfiling bool
pkg/apis/componentconfig/types.go:	EnableProfiling bool
pkg/apis/componentconfig/types.go:	EnableContentionProfiling bool
pkg/apis/componentconfig/types.go:	EnableGarbageCollector bool
pkg/apis/componentconfig/types.go:	DisableAttachDetachReconcilerSync bool
pkg/apis/componentconfig/types.go:	EnableTaintManager bool
pkg/apis/componentconfig/types.go:	EnableHostPathProvisioning bool
pkg/apis/componentconfig/types.go:	EnableDynamicProvisioning bool
pkg/apis/componentconfig/v1alpha1/types.go:	EnableProfiling *bool `json:"enableProfiling"`
pkg/apis/componentconfig/v1alpha1/types.go:	EnableContentionProfiling bool `json:"enableContentionProfiling"`
pkg/apis/componentconfig/v1alpha1/types.go:	EnableServer *bool `json:"enableServer"`
pkg/apis/componentconfig/v1alpha1/types.go:	EnableDebuggingHandlers *bool `json:"enableDebuggingHandlers"`
pkg/apis/componentconfig/v1alpha1/types.go:	EnableContentionProfiling bool `json:"enableContentionProfiling"`
pkg/apis/componentconfig/v1alpha1/types.go:	RegisterSchedulable *bool `json:"registerSchedulable"`
pkg/apis/componentconfig/v1alpha1/types.go:	EnableCustomMetrics bool `json:"enableCustomMetrics"`
pkg/apis/componentconfig/v1alpha1/types.go:	EnableControllerAttachDetach *bool `json:"enableControllerAttachDetach"`
pkg/apis/componentconfig/v1alpha1/types.go:	MakeIPTablesUtilChains *bool `json:"makeIPTablesUtilChains"`
pkg/apis/componentconfig/v1alpha1/types.go:	EnableCRI *bool `json:"enableCRI,omitempty"`
pkg/apis/componentconfig/v1alpha1/types.go:	DockerEnableSharedPID *bool `json:"dockerEnableSharedPID,omitempty"`
pkg/apis/componentconfig/v1alpha1/types.go:	ExperimentalNodeAllocatableIgnoreEvictionThreshold bool `json:"experimentalNodeAllocatableIgnoreEvictionThreshold,omitempty"`
pkg/apis/componentconfig/v1alpha1/types.go:	Enabled *bool `json:"enabled"`
pkg/apis/componentconfig/v1alpha1/types.go:	Enabled *bool `json:"enabled"`

but all the examples seem to be from "componentconfig" and most of those are using the word "Enable".

From that I might infer a rule about "Enable", but not about "-able".

[smarterclayton]

This is 100% in line with existing guidance. LGTM, although any additions you want to add as per request are fine.

[smarterclayton]

@erictune I thought the point of the comment was "never use Is*", which is true. Not the "*able" part, which is sometimes true.

[pmorie]

The point was: avoid Is as a prefix for boolean fields.

Audit with:

$ find pkg/api{,s} -name types.go | xargs grep bool

[smarterclayton]

That also goes for go coding style and general conventions. We may use Is* in a few function names, but generally Go style frowns on the redundant verb.

[duglin]

Not to bikeshed :-) but....

• if add Is is redudant then I would think adding Num is too for int's. Yet we have those.
• allowing your choice of programming language to define what gets externalized (e.g. APIs, error messages, etc..) is not a good idea - it means pain later on if you ever change the lang, and not to mention interop since not everyone will use that lang.

[smarterclayton]

if add Is is redudant then I would think adding Num is too for int's. Yet we have those.

Num describes fields that were confusing to users on first glance because they could be confused with a declarative action (NumUnreadyPods is strictly better than UnreadyPods).

allowing your choice of programming language to define what gets externalized (e.g. APIs, error messages, etc..) is not a good idea - it means pain later on if you ever change the lang, and not to mention interop since not everyone will use that lang.

The two have nothing to do with each other. I was just reiterating that this is also something we apply to coding conventions.

Any other comments?

[duglin]

nope - not from me

[thockin]

I am skeptical of any bool field. They often end up wanting to be enums. The fact that we have none today is pretty telling, IMO. I don't have a problem with this PR, but I'd like to guide people away from bool fields in most cases.

…

On Mon, May 15, 2017 at 4:30 PM, Doug Davis ***@***.***> wrote: nope - not from me — You are receiving this because you are on a team that was mentioned. Reply to this email directly, view it on GitHub <#624 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFVgVFGee4jB9GKsWVQY88xxtSEpCu2sks5r6OAsgaJpZM4NbqtD> .

[pmorie]

The fact that we have none today is pretty telling, IMO.

We have tens of them - do we have a mismatch?

[smarterclayton]

Merging based on no disagreement, Tim will record his philosophical Distaste|Disdain for booleans in a follow on PR :)